Skip to content

Commit

Permalink
Address PR comments
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 17, 2024
1 parent e53536b commit 9dc9ccb
Showing 1 changed file with 33 additions and 11 deletions.
44 changes: 33 additions & 11 deletions go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"math/rand"
"sync"
"sync/atomic"
"time"

"google.golang.org/protobuf/encoding/prototext"
Expand Down Expand Up @@ -185,6 +186,9 @@ type txThrottlerStateImpl struct {

healthCheck discovery.LegacyHealthCheck
topologyWatchers []TopologyWatcherInterface

shardMaxLag atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Static Code Checks Etc

Int64 not declared by package atomic (typecheck)

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Region Sharding example using etcd on ubuntu-latest

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Local example using k8s on ubuntu-latest

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Local example using consul on ubuntu-latest

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Local example using etcd on ubuntu-latest

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64

Check failure on line 190 in go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: atomic.Int64
endChannel chan bool
}

// NewTxThrottler tries to construct a txThrottler from the
Expand Down Expand Up @@ -330,8 +334,9 @@ func newTxThrottlerState(topoServer *topo.Server, config *txThrottlerConfig, tar
return nil, err
}
result := &txThrottlerStateImpl{
config: config,
throttler: t,
config: config,
throttler: t,
endChannel: make(chan bool),
}
result.healthCheck = healthCheckFactory()
result.healthCheck.SetListener(result, false /* sendDownEvents */)
Expand All @@ -349,6 +354,9 @@ func newTxThrottlerState(topoServer *topo.Server, config *txThrottlerConfig, tar
discovery.DefaultTopologyWatcherRefreshInterval,
discovery.DefaultTopoReadConcurrency))
}

go result.updateMaxShardLag()

return result, nil
}

Expand All @@ -361,18 +369,31 @@ func (ts *txThrottlerStateImpl) throttle() bool {
ts.throttleMu.Lock()
defer ts.throttleMu.Unlock()

var maxLag uint32

for _, tabletType := range ts.config.tabletTypes {
maxLagPerTabletType := ts.throttler.LastMaxLagNotIgnoredForTabletType(tabletType)
if maxLagPerTabletType > maxLag {
maxLag = maxLagPerTabletType
}
}
maxLag := ts.shardMaxLag.Load()

return ts.throttler.Throttle(0 /* threadId */) > 0 &&
int64(maxLag) > ts.config.throttlerConfig.TargetReplicationLagSec
maxLag > ts.config.throttlerConfig.TargetReplicationLagSec
}

func (ts *txThrottlerStateImpl) updateMaxShardLag() {
// 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.throttlerConfig.TargetReplicationLagSec/2) * time.Second)
for {
select {
case _ = <-ticker.C:
var maxLag uint32

for _, tabletType := range ts.config.tabletTypes {
maxLagPerTabletType := ts.throttler.LastMaxLagNotIgnoredForTabletType(tabletType)
if maxLagPerTabletType > maxLag {
maxLag = maxLagPerTabletType
}
}
ts.shardMaxLag.Store(int64(maxLag))
case _ = <-ts.endChannel:
break
}
}
}

func (ts *txThrottlerStateImpl) deallocateResources() {
Expand All @@ -387,6 +408,7 @@ func (ts *txThrottlerStateImpl) deallocateResources() {
ts.healthCheck.Close()
ts.healthCheck = nil

ts.endChannel <- true
// After ts.healthCheck is closed txThrottlerStateImpl.StatsUpdate() is guaranteed not
// to be executing, so we can safely close the throttler.
ts.throttler.Close()
Expand Down

0 comments on commit 9dc9ccb

Please sign in to comment.