diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go index 6b63dbea2e8..0fceb175e0b 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go @@ -251,7 +251,7 @@ func (t *txThrottler) Throttle(priority int, workload string) (result bool) { // Throttle according to both what the throttler state says and the priority. Workloads with lower priority value // are less likely to be throttled. - result = t.state.throttle() && rand.Intn(sqlparser.MaxPriorityValue) < priority + result = rand.Intn(sqlparser.MaxPriorityValue) < priority && t.state.throttle() t.requestsTotal.Add(workload, 1) if result { @@ -368,14 +368,15 @@ func (ts *txThrottlerStateImpl) throttle() bool { maxLag := atomic.LoadInt64(&ts.shardMaxLag) - return ts.throttler.Throttle(0 /* threadId */) > 0 && - maxLag > ts.config.TxThrottlerConfig.TargetReplicationLagSec + return maxLag > ts.config.TxThrottlerConfig.TargetReplicationLagSec && + ts.throttler.Throttle(0 /* threadId */) > 0 } func (ts *txThrottlerStateImpl) updateMaxShardLag() { defer ts.endWaitGroup.Done() // We use half of the target lag to ensure we have enough resolution to see changes in lag below that value ticker := time.NewTicker(time.Duration(ts.config.TxThrottlerConfig.TargetReplicationLagSec/2) * time.Second) + defer ticker.Stop() outerloop: for { select { diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go index 3e465d9ea81..3f10530110a 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go @@ -108,14 +108,12 @@ func TestEnabledThrottler(t *testing.T) { calls = append(calls, call) // 3 - call = mockThrottler.EXPECT().Throttle(0) - call.Return(1 * time.Second) - calls = append(calls, call) + // Nothing gets mocked here because the order of evaluation in txThrottler.Throttle() evaluates first + // whether the priority allows for throttling or not, so no need to mock calls in mockThrottler.Throttle() // 4 - call = mockThrottler.EXPECT().Throttle(0) - call.Return(1 * time.Second) - calls = append(calls, call) + // Nothing gets mocked here because the order of evaluation in txThrottlerStateImpl.Throttle() evaluates first + // whether there is lag or not, so no call to the underlying mockThrottler is issued. call = mockThrottler.EXPECT().Close() calls = append(calls, call)