Skip to content

Commit

Permalink
Replace aotmic.Int64 with atomic.(Load|Store)Int64 to avoid CI issues
Browse files Browse the repository at this point in the history
Signed-off-by: Eduardo J. Ortega U <[email protected]>
  • Loading branch information
ejortegau committed Jan 18, 2024
1 parent 01095bc commit 0f40692
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
6 changes: 3 additions & 3 deletions go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ type txThrottlerStateImpl struct {
healthCheck discovery.LegacyHealthCheck
topologyWatchers []TopologyWatcherInterface

shardMaxLag atomic.Int64
shardMaxLag int64
endChannel chan bool
}

Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
8 changes: 6 additions & 2 deletions go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"])
Expand All @@ -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"])
Expand Down

0 comments on commit 0f40692

Please sign in to comment.