Skip to content

Commit 2e9913f

Browse files
committed
catalog/lease: fix race condition with testing knob
Previously, the testing knob TestingDisableRangeFeedCheckpoint was read without any mutex, which could lead to a race condition in certain tests as detected by the race detector. To address this, this patch moves the field into the lease manager as an atomic, since testing knobs are copied, so they cannot store mutexes or atomics. Fixes: #156182 Release note: None
1 parent 1e00b30 commit 2e9913f

File tree

2 files changed

+8
-6
lines changed

2 files changed

+8
-6
lines changed

pkg/sql/catalog/lease/lease.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,11 @@ type Manager struct {
15671567
bytesMonitor *mon.BytesMonitor
15681568
// boundAccount tracks the memory usage from leased descriptors.
15691569
boundAccount *mon.ConcurrentBoundAccount
1570+
1571+
// TestingDisableRangeFeedCheckpoint is used to disable rangefeed checkpoints.
1572+
// Ideally, this would be inside StorageTestingKnobs, but those get copied into
1573+
// the lease manager.
1574+
TestingDisableRangeFeedCheckpoint atomic.Bool
15701575
}
15711576

15721577
const leaseConcurrencyLimit = 5
@@ -2506,7 +2511,7 @@ func (m *Manager) watchForUpdates(ctx context.Context) {
25062511
ctx context.Context, ev *kvpb.RangeFeedValue,
25072512
) {
25082513
// Skip updates if rangefeeds are disabled.
2509-
if m.testingKnobs.DisableRangeFeedCheckpoint {
2514+
if m.TestingDisableRangeFeedCheckpoint.Load() {
25102515
return
25112516
}
25122517
// Check first if it is the special descriptor metadata key used to track
@@ -2572,7 +2577,7 @@ func (m *Manager) watchForUpdates(ctx context.Context) {
25722577
}
25732578

25742579
handleCheckpoint := func(ctx context.Context, checkpoint *kvpb.RangeFeedCheckpoint) {
2575-
if m.testingKnobs.DisableRangeFeedCheckpoint {
2580+
if m.TestingDisableRangeFeedCheckpoint.Load() {
25762581
return
25772582
}
25782583
// Track checkpoints that occur from the rangefeed to make sure progress
@@ -3150,7 +3155,7 @@ func (m *Manager) TestingSetDisableRangeFeedCheckpointFn(disable bool) chan stru
31503155
m.mu.Lock()
31513156
defer m.mu.Unlock()
31523157
m.mu.rangeFeedCheckpoints = 0
3153-
m.testingKnobs.DisableRangeFeedCheckpoint = disable
3158+
m.TestingDisableRangeFeedCheckpoint.Store(disable)
31543159
if disable {
31553160
m.testingKnobs.RangeFeedResetChannel = make(chan struct{})
31563161
} else {

pkg/sql/catalog/lease/testutils.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ type ManagerTestingKnobs struct {
6161
// To disable the deletion of orphaned leases at server startup.
6262
DisableDeleteOrphanedLeases bool
6363

64-
// DisableRangeFeedCheckpoint is used to disable rangefeed checkpoints.
65-
DisableRangeFeedCheckpoint bool
66-
6764
// RangeFeedReset channel is closed to indicate that the range feed
6865
// has been reset.
6966
RangeFeedResetChannel chan struct{}

0 commit comments

Comments
 (0)