diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go index 22b2c0799fc..620656bfa6a 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go @@ -187,7 +187,7 @@ type txThrottlerStateImpl struct { healthCheck discovery.LegacyHealthCheck topologyWatchers []TopologyWatcherInterface - shardMaxLag atomic.Int64 + shardMaxLag int64 endChannel chan bool } @@ -369,7 +369,7 @@ func (ts *txThrottlerStateImpl) throttle() bool { ts.throttleMu.Lock() defer ts.throttleMu.Unlock() - maxLag := ts.shardMaxLag.Load() + maxLag := atomic.LoadInt64(&ts.shardMaxLag) return ts.throttler.Throttle(0 /* threadId */) > 0 && maxLag > ts.config.throttlerConfig.TargetReplicationLagSec @@ -389,7 +389,7 @@ func (ts *txThrottlerStateImpl) updateMaxShardLag() { maxLag = maxLagPerTabletType } } - ts.shardMaxLag.Store(int64(maxLag)) + atomic.StoreInt64(&ts.shardMaxLag, int64(maxLag)) case _ = <-ts.endChannel: break } diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go index b23c1a2b062..8311638e017 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go @@ -22,6 +22,7 @@ package txthrottler //go:generate mockgen -destination mock_topology_watcher_test.go -package txthrottler vitess.io/vitess/go/vt/vttablet/tabletserver/txthrottler TopologyWatcherInterface import ( + "sync/atomic" "testing" "time" @@ -148,9 +149,12 @@ func TestEnabledThrottler(t *testing.T) { throttlerImpl, ok := throttler.state.(*txThrottlerStateImpl) assert.True(t, ok) + // Stop the go routine that keeps updating the cached shard's max lag to preventi it from changing the value in a + // way that will interfere with how we manipulate that value in our tests to evaluate different cases: + throttlerImpl.endChannel <- true // 1 should not throttle due to return value of underlying Throttle(), despite high lag - throttlerImpl.shardMaxLag.Store(20) + atomic.StoreInt64(&throttlerImpl.shardMaxLag, 20) assert.False(t, throttler.Throttle(100, "some-workload")) assert.Equal(t, int64(1), throttler.requestsTotal.Counts()["some-workload"]) assert.Zero(t, throttler.requestsThrottled.Counts()["some-workload"]) @@ -175,7 +179,7 @@ func TestEnabledThrottler(t *testing.T) { assert.Equal(t, int64(1), throttler.requestsThrottled.Counts()["some-workload"]) // 4 should not throttle despite return value of underlying Throttle() and priority = 100, due to low lag - throttlerImpl.shardMaxLag.Store(1) + atomic.StoreInt64(&throttlerImpl.shardMaxLag, 1) assert.False(t, throttler.Throttle(100, "some-workload")) assert.Equal(t, int64(4), throttler.requestsTotal.Counts()["some-workload"]) assert.Equal(t, int64(1), throttler.requestsThrottled.Counts()["some-workload"])