Skip to content

Commit

Permalink
sql/gcjob/gcjobnotifier: fix rare panic
Browse files Browse the repository at this point in the history
If a notifier is added before the initial system config, it can result in a
crash. This may be more common in 22.1, where the initial config takes longer
to be populated.

Fixes #77425

Release note (bug fix): Fixes a rare crash which can occur when restarting a
node after dropping tables.
  • Loading branch information
ajwerner committed Apr 26, 2022
1 parent 976853d commit 4800a2e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
7 changes: 5 additions & 2 deletions pkg/sql/gcjob/gcjobnotifier/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,11 @@ func (n *Notifier) AddNotifyee(ctx context.Context) (onChange <-chan struct{}, c
if n.mu.deltaFilter == nil {
zoneCfgFilter := gossip.MakeSystemConfigDeltaFilter(n.prefix)
n.mu.deltaFilter = &zoneCfgFilter
// Initialize the filter with the current values.
n.mu.deltaFilter.ForModified(n.provider.GetSystemConfig(), func(kv roachpb.KeyValue) {})
// Initialize the filter with the current values, if they exist.
cfg := n.provider.GetSystemConfig()
if cfg != nil {
n.mu.deltaFilter.ForModified(cfg, func(kv roachpb.KeyValue) {})
}
}
c := make(chan struct{}, 1)
n.mu.notifyees[c] = struct{}{}
Expand Down
17 changes: 17 additions & 0 deletions pkg/sql/gcjob/gcjobnotifier/notifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@ func TestNotifier(t *testing.T) {
n.AddNotifyee(ctx)
})
})
t.Run("safe to on AddNotifyee after start before config", func(t *testing.T) {
ch := make(chan struct{}, 1)
cfg := mkSystemConfig(mkZoneConfigKV(1, 1, "1"))
p := &testingProvider{ch: ch}
n := New(settings, p, keys.SystemSQLCodec, stopper)
n.Start(ctx)
n1Ch, cleanup := n.AddNotifyee(ctx)
defer cleanup()
select {
case <-time.After(10 * time.Millisecond):
case <-n1Ch:
t.Fatal("should not have gotten notified")
}
p.setSystemConfig(cfg)
ch <- struct{}{}
<-n1Ch
})
t.Run("notifies on changed delta and cleanup", func(t *testing.T) {
cfg := config.NewSystemConfig(zonepb.DefaultSystemZoneConfigRef())
cfg.Values = []roachpb.KeyValue{
Expand Down

0 comments on commit 4800a2e

Please sign in to comment.