From 6eec92f7de3b73e292b248ce70545252c184d6f5 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Thu, 18 May 2023 22:58:04 +0200 Subject: [PATCH] txthrottler: verify config at vttablet startup, consolidate funcs Signed-off-by: Tim Vaillancourt --- go/flags/endtoend/vttablet.txt | 3 +- .../vttablet/tabletserver/tabletenv/config.go | 61 ++++++++++---- .../tabletserver/tabletenv/config_test.go | 84 +++++++++++++++++-- .../tabletserver/txthrottler/tx_throttler.go | 74 +++++----------- .../txthrottler/tx_throttler_test.go | 74 ++++++++-------- 5 files changed, 183 insertions(+), 113 deletions(-) diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index 939ec91c1e9..2b2482215e6 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/go/flags/endtoend/vttablet.txt @@ -345,11 +345,10 @@ Usage of vttablet: --twopc_abandon_age float time in seconds. Any unresolved transaction older than this time will be sent to the coordinator to be resolved. --twopc_coordinator_address string address of the (VTGate) process(es) that will be used to notify of abandoned transactions. --twopc_enable if the flag is on, 2pc is enabled. Other 2pc flags must be supplied. - --tx-throttler-config string Synonym to -tx_throttler_config (default "target_replication_lag_sec: 2\nmax_replication_lag_sec: 10\ninitial_rate: 100\nmax_increase: 1\nemergency_decrease: 0.5\nmin_duration_between_increases_sec: 40\nmax_duration_between_increases_sec: 62\nmin_duration_between_decreases_sec: 20\nspread_backlog_across_sec: 20\nage_bad_rate_after_sec: 180\nbad_rate_increase: 0.1\nmax_rate_approach_threshold: 0.9\n") --tx-throttler-default-priority int Default priority assigned to queries that lack priority information (default 100) --tx-throttler-healthcheck-cells strings Synonym to -tx_throttler_healthcheck_cells --tx-throttler-tablet-types strings A comma-separated list of tablet types. Only tablets of this type are monitored for replication lag by the transaction throttler. Supported types are replica and/or rdonly. (default replica) - --tx_throttler_config string The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message. (default "target_replication_lag_sec: 2\nmax_replication_lag_sec: 10\ninitial_rate: 100\nmax_increase: 1\nemergency_decrease: 0.5\nmin_duration_between_increases_sec: 40\nmax_duration_between_increases_sec: 62\nmin_duration_between_decreases_sec: 20\nspread_backlog_across_sec: 20\nage_bad_rate_after_sec: 180\nbad_rate_increase: 0.1\nmax_rate_approach_threshold: 0.9\n") + --tx_throttler_config string The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message. (default "target_replication_lag_sec:2 max_replication_lag_sec:10 initial_rate:100 max_increase:1 emergency_decrease:0.5 min_duration_between_increases_sec:40 max_duration_between_increases_sec:62 min_duration_between_decreases_sec:20 spread_backlog_across_sec:20 age_bad_rate_after_sec:180 bad_rate_increase:0.1 max_rate_approach_threshold:0.9") --tx_throttler_healthcheck_cells strings A comma-separated list of cells. Only tabletservers running in these cells will be monitored for replication lag by the transaction throttler. --unhealthy_threshold duration replication lag after which a replica is considered unhealthy (default 2h0m0s) --v Level log level for V logs diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index 4ecb42655aa..34e3ed44147 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -31,14 +31,16 @@ import ( "vitess.io/vitess/go/streamlog" "vitess.io/vitess/go/vt/dbconfigs" "vitess.io/vitess/go/vt/log" - querypb "vitess.io/vitess/go/vt/proto/query" - topodatapb "vitess.io/vitess/go/vt/proto/topodata" - vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/throttler" "vitess.io/vitess/go/vt/topo/topoproto" "vitess.io/vitess/go/vt/vterrors" + + querypb "vitess.io/vitess/go/vt/proto/query" + throttlerdatapb "vitess.io/vitess/go/vt/proto/throttlerdata" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) // These constants represent values for various config parameters. @@ -89,6 +91,24 @@ var ( txLogHandler = "/debug/txlog" ) +type TxThrottlerConfigFlag struct { + *throttlerdatapb.Configuration +} + +func NewTxThrottlerConfigFlag() *TxThrottlerConfigFlag { + return &TxThrottlerConfigFlag{&throttlerdatapb.Configuration{}} +} + +func (t *TxThrottlerConfigFlag) Get() *throttlerdatapb.Configuration { + return t.Configuration +} + +func (t *TxThrottlerConfigFlag) Set(arg string) error { + return prototext.Unmarshal([]byte(arg), t) +} + +func (t *TxThrottlerConfigFlag) Type() string { return "string" } + // RegisterTabletEnvFlags is a public API to register tabletenv flags for use by test cases that expect // some flags to be set with default values func RegisterTabletEnvFlags(fs *pflag.FlagSet) { @@ -158,7 +178,7 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { SecondsVar(fs, ¤tConfig.TwoPCAbandonAge, "twopc_abandon_age", defaultConfig.TwoPCAbandonAge, "time in seconds. Any unresolved transaction older than this time will be sent to the coordinator to be resolved.") // Tx throttler config flagutil.DualFormatBoolVar(fs, ¤tConfig.EnableTxThrottler, "enable_tx_throttler", defaultConfig.EnableTxThrottler, "If true replication-lag-based throttling on transactions will be enabled.") - flagutil.DualFormatStringVar(fs, ¤tConfig.TxThrottlerConfig, "tx_throttler_config", defaultConfig.TxThrottlerConfig, "The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message.") + fs.Var(currentConfig.TxThrottlerConfig, "tx_throttler_config", "The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message.") flagutil.DualFormatStringListVar(fs, ¤tConfig.TxThrottlerHealthCheckCells, "tx_throttler_healthcheck_cells", defaultConfig.TxThrottlerHealthCheckCells, "A comma-separated list of cells. Only tabletservers running in these cells will be monitored for replication lag by the transaction throttler.") fs.IntVar(¤tConfig.TxThrottlerDefaultPriority, "tx-throttler-default-priority", defaultConfig.TxThrottlerDefaultPriority, "Default priority assigned to queries that lack priority information") fs.Var(currentConfig.TxThrottlerTabletTypes, "tx-throttler-tablet-types", "A comma-separated list of tablet types. Only tablets of this type are monitored for replication lag by the transaction throttler. Supported types are replica and/or rdonly.") @@ -340,7 +360,7 @@ type TabletConfig struct { TwoPCAbandonAge Seconds `json:"-"` EnableTxThrottler bool `json:"-"` - TxThrottlerConfig string `json:"-"` + TxThrottlerConfig *TxThrottlerConfigFlag `json:"-"` TxThrottlerHealthCheckCells []string `json:"-"` TxThrottlerDefaultPriority int `json:"-"` TxThrottlerTabletTypes *topoproto.TabletTypeListFlag `json:"-"` @@ -657,9 +677,6 @@ func (c *TabletConfig) Verify() error { if v := c.HotRowProtection.MaxConcurrency; v <= 0 { return fmt.Errorf("--hot_row_protection_concurrent_transactions must be > 0 (specified value: %v)", v) } - if v := c.TxThrottlerDefaultPriority; v > sqlparser.MaxPriorityValue || v < 0 { - return fmt.Errorf("--tx-throttler-default-priority must be > 0 and < 100 (specified value: %d)", v) - } return nil } @@ -695,6 +712,22 @@ func (c *TabletConfig) verifyTransactionLimitConfig() error { // verifyTxThrottlerConfig checks the TxThrottler related config for sanity. func (c *TabletConfig) verifyTxThrottlerConfig() error { + if !c.EnableTxThrottler { + return nil + } + + err := throttler.MaxReplicationLagModuleConfig{Configuration: c.TxThrottlerConfig.Get()}.Verify() + if err != nil { + return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "failed to parse throttlerdatapb.Configuration config: %v", err) + } + + if len(c.TxThrottlerHealthCheckCells) == 0 { + return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "empty healthCheckCells given: %+v", c.TxThrottlerHealthCheckCells) + } + if v := c.TxThrottlerDefaultPriority; v > sqlparser.MaxPriorityValue || v < 0 { + return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "--tx-throttler-default-priority must be > 0 and < 100 (specified value: %d)", v) + } + if c.TxThrottlerTabletTypes == nil || len(*c.TxThrottlerTabletTypes) == 0 { return vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "--tx-throttler-tablet-types must be defined when transaction throttler is enabled") } @@ -706,6 +739,7 @@ func (c *TabletConfig) verifyTxThrottlerConfig() error { return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported tablet type %q", tabletType) } } + return nil } @@ -812,17 +846,16 @@ var defaultConfig = TabletConfig{ EnablePerWorkloadTableMetrics: false, } -// defaultTxThrottlerConfig formats the default throttlerdata.Configuration -// object in text format. It uses the object returned by -// throttler.DefaultMaxReplicationLagModuleConfig().Configuration and overrides some of its -// fields. It panics on error. -func defaultTxThrottlerConfig() string { +// defaultTxThrottlerConfig returns the default TxThrottlerConfigFlag object based on +// a throttler.DefaultMaxReplicationLagModuleConfig().Configuration and overrides some of +// its fields. It panics on error. +func defaultTxThrottlerConfig() *TxThrottlerConfigFlag { // Take throttler.DefaultMaxReplicationLagModuleConfig and override some fields. config := throttler.DefaultMaxReplicationLagModuleConfig().Configuration // TODO(erez): Make DefaultMaxReplicationLagModuleConfig() return a MaxReplicationLagSec of 10 // and remove this line. config.MaxReplicationLagSec = 10 - return prototext.Format(config) + return &TxThrottlerConfigFlag{config} } func defaultTransactionLimitConfig() TransactionLimitConfig { diff --git a/go/vt/vttablet/tabletserver/tabletenv/config_test.go b/go/vt/vttablet/tabletserver/tabletenv/config_test.go index 0b1bd707de0..362c36ea93a 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config_test.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config_test.go @@ -26,11 +26,13 @@ import ( "vitess.io/vitess/go/test/utils" "vitess.io/vitess/go/vt/dbconfigs" - topodatapb "vitess.io/vitess/go/vt/proto/topodata" - vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/throttler" "vitess.io/vitess/go/vt/topo/topoproto" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/yaml2" + + topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) func TestConfigParse(t *testing.T) { @@ -332,13 +334,63 @@ func TestFlags(t *testing.T) { assert.Equal(t, want, currentConfig) } +func TestTxThrottlerConfigFlag(t *testing.T) { + f := NewTxThrottlerConfigFlag() + defaultMaxReplicationLagModuleConfig := throttler.DefaultMaxReplicationLagModuleConfig().Configuration + + { + assert.Nil(t, f.Set(defaultMaxReplicationLagModuleConfig.String())) + assert.Equal(t, defaultMaxReplicationLagModuleConfig.String(), f.String()) + assert.Equal(t, "string", f.Type()) + } + { + defaultMaxReplicationLagModuleConfig.TargetReplicationLagSec = 5 + assert.Nil(t, f.Set(defaultMaxReplicationLagModuleConfig.String())) + assert.NotNil(t, f.Get()) + assert.Equal(t, int64(5), f.Get().TargetReplicationLagSec) + } + { + assert.NotNil(t, f.Set("should not parse")) + } +} + func TestVerifyTxThrottlerConfig(t *testing.T) { + defaultMaxReplicationLagModuleConfig := throttler.DefaultMaxReplicationLagModuleConfig().Configuration + { - // default config (replica) + // default (disabled) assert.Nil(t, currentConfig.verifyTxThrottlerConfig()) } { - // replica + rdonly (allowed) + // enabled without throttler config + currentConfig.EnableTxThrottler = true + assert.NotNil(t, currentConfig.verifyTxThrottlerConfig()) + err := currentConfig.verifyTxThrottlerConfig() + assert.NotNil(t, err) + assert.Equal(t, vtrpcpb.Code_FAILED_PRECONDITION, vterrors.Code(err)) + } + { + // enabled without cells defined + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + err := currentConfig.verifyTxThrottlerConfig() + assert.NotNil(t, err) + assert.Equal(t, vtrpcpb.Code_FAILED_PRECONDITION, vterrors.Code(err)) + } + { + // enabled with good config (default/replica tablet type) + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + currentConfig.TxThrottlerConfig.TargetReplicationLagSec = 5 + currentConfig.TxThrottlerHealthCheckCells = []string{"cell1"} + assert.Nil(t, currentConfig.verifyTxThrottlerConfig()) + } + { + // enabled + replica and rdonly tablet types + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + currentConfig.TxThrottlerConfig.TargetReplicationLagSec = 5 + currentConfig.TxThrottlerHealthCheckCells = []string{"cell1"} currentConfig.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{ topodatapb.TabletType_REPLICA, topodatapb.TabletType_RDONLY, @@ -346,17 +398,37 @@ func TestVerifyTxThrottlerConfig(t *testing.T) { assert.Nil(t, currentConfig.verifyTxThrottlerConfig()) } { - // no tablet types + // enabled without tablet types + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + currentConfig.TxThrottlerConfig.TargetReplicationLagSec = 5 + currentConfig.TxThrottlerHealthCheckCells = []string{"cell1"} currentConfig.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{} err := currentConfig.verifyTxThrottlerConfig() assert.NotNil(t, err) assert.Equal(t, vtrpcpb.Code_FAILED_PRECONDITION, vterrors.Code(err)) } { - // disallowed tablet type + // enabled + disallowed tablet type + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + currentConfig.TxThrottlerConfig.TargetReplicationLagSec = 5 + currentConfig.TxThrottlerHealthCheckCells = []string{"cell1"} currentConfig.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{topodatapb.TabletType_DRAINED} err := currentConfig.verifyTxThrottlerConfig() assert.NotNil(t, err) assert.Equal(t, vtrpcpb.Code_INVALID_ARGUMENT, vterrors.Code(err)) } + { + // enabled + disallowed priority + currentConfig.EnableTxThrottler = true + currentConfig.TxThrottlerConfig = &TxThrottlerConfigFlag{defaultMaxReplicationLagModuleConfig} + currentConfig.TxThrottlerConfig.TargetReplicationLagSec = 5 + currentConfig.TxThrottlerHealthCheckCells = []string{"cell1"} + currentConfig.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{topodatapb.TabletType_REPLICA} + currentConfig.TxThrottlerDefaultPriority = 12345 + err := currentConfig.verifyTxThrottlerConfig() + assert.NotNil(t, err) + assert.Equal(t, vtrpcpb.Code_INVALID_ARGUMENT, vterrors.Code(err)) + } } diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go index bc5235593ac..5af00a1d087 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go @@ -18,13 +18,11 @@ package txthrottler import ( "context" - "fmt" "math/rand" "strings" "sync" "time" - "google.golang.org/protobuf/encoding/prototext" "google.golang.org/protobuf/proto" "vitess.io/vitess/go/stats" @@ -186,64 +184,36 @@ type txThrottlerState struct { // This function calls tryCreateTxThrottler that does the actual creation work // and returns an error if one occurred. func NewTxThrottler(env tabletenv.Env, topoServer *topo.Server) TxThrottler { - txThrottler, err := tryCreateTxThrottler(env, topoServer) - if err != nil { - log.Errorf("Error creating transaction throttler. Transaction throttling will"+ - " be disabled. Error: %v", err) - // newTxThrottler with disabled config never returns an error - txThrottler, _ = newTxThrottler(env, topoServer, &txThrottlerConfig{enabled: false}) - } else { - log.Infof("Initialized transaction throttler with config: %+v", txThrottler.config) - } - return txThrottler -} - -// InitDBConfig initializes the target parameters for the throttler. -func (t *txThrottler) InitDBConfig(target *querypb.Target) { - t.target = proto.Clone(target).(*querypb.Target) -} - -func tryCreateTxThrottler(env tabletenv.Env, topoServer *topo.Server) (*txThrottler, error) { - if !env.Config().EnableTxThrottler { - return newTxThrottler(env, topoServer, &txThrottlerConfig{enabled: false}) - } + throttlerConfig := &txThrottlerConfig{enabled: false} + + if env.Config().EnableTxThrottler { + // Clone tsv.TxThrottlerHealthCheckCells so that we don't assume tsv.TxThrottlerHealthCheckCells + // is immutable. + healthCheckCells := make([]string, len(env.Config().TxThrottlerHealthCheckCells)) + copy(healthCheckCells, env.Config().TxThrottlerHealthCheckCells) + + throttlerConfig = &txThrottlerConfig{ + enabled: true, + tabletTypes: env.Config().TxThrottlerTabletTypes, + throttlerConfig: env.Config().TxThrottlerConfig.Get(), + healthCheckCells: healthCheckCells, + } - var throttlerConfig throttlerdatapb.Configuration - if err := prototext.Unmarshal([]byte(env.Config().TxThrottlerConfig), &throttlerConfig); err != nil { - return nil, err + defer log.Infof("Initialized transaction throttler with config: %+v", throttlerConfig) } - // Clone tsv.TxThrottlerHealthCheckCells so that we don't assume tsv.TxThrottlerHealthCheckCells - // is immutable. - healthCheckCells := make([]string, len(env.Config().TxThrottlerHealthCheckCells)) - copy(healthCheckCells, env.Config().TxThrottlerHealthCheckCells) - - return newTxThrottler(env, topoServer, &txThrottlerConfig{ - enabled: true, - tabletTypes: env.Config().TxThrottlerTabletTypes, - throttlerConfig: &throttlerConfig, - healthCheckCells: healthCheckCells, - }) -} - -func newTxThrottler(env tabletenv.Env, topoServer *topo.Server, config *txThrottlerConfig) (*txThrottler, error) { - if config.enabled { - // Verify config. - err := throttler.MaxReplicationLagModuleConfig{Configuration: config.throttlerConfig}.Verify() - if err != nil { - return nil, err - } - if len(config.healthCheckCells) == 0 { - return nil, fmt.Errorf("empty healthCheckCells given. %+v", config) - } - } return &txThrottler{ - config: config, + config: throttlerConfig, topoServer: topoServer, throttlerRunning: env.Exporter().NewGauge("TransactionThrottlerRunning", "transaction throttler running state"), requestsTotal: env.Exporter().NewCounter("TransactionThrottlerRequests", "transaction throttler requests"), requestsThrottled: env.Exporter().NewCounter("TransactionThrottlerThrottled", "transaction throttler requests throttled"), - }, nil + } +} + +// InitDBConfig initializes the target parameters for the throttler. +func (t *txThrottler) InitDBConfig(target *querypb.Target) { + t.target = proto.Clone(target).(*querypb.Target) } // Open opens the transaction throttler. It must be called prior to 'Throttle'. diff --git a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go index 97138e3928c..ffb88bf21f6 100644 --- a/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go +++ b/go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go @@ -36,7 +36,6 @@ import ( "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" querypb "vitess.io/vitess/go/vt/proto/query" - throttlerdatapb "vitess.io/vitess/go/vt/proto/throttlerdata" topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) @@ -116,38 +115,39 @@ func TestEnabledThrottler(t *testing.T) { config.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{topodatapb.TabletType_REPLICA} env := tabletenv.NewEnv(config, t.Name()) - throttler, err := tryCreateTxThrottler(env, ts) - assert.Nil(t, err) + throttler := NewTxThrottler(env, ts) + throttlerImpl, _ := throttler.(*txThrottler) + assert.NotNil(t, throttlerImpl) throttler.InitDBConfig(&querypb.Target{ Keyspace: "keyspace", Shard: "shard", }) - assert.Nil(t, throttler.Open()) - assert.Equal(t, int64(1), throttler.throttlerRunning.Get()) + assert.Nil(t, throttlerImpl.Open()) + assert.Equal(t, int64(1), throttlerImpl.throttlerRunning.Get()) - assert.False(t, throttler.Throttle(100)) - assert.Equal(t, int64(1), throttler.requestsTotal.Get()) - assert.Zero(t, throttler.requestsThrottled.Get()) + assert.False(t, throttlerImpl.Throttle(100)) + assert.Equal(t, int64(1), throttlerImpl.requestsTotal.Get()) + assert.Zero(t, throttlerImpl.requestsThrottled.Get()) - throttler.state.StatsUpdate(tabletStats) // This calls replication lag thing + throttlerImpl.state.StatsUpdate(tabletStats) // This calls replication lag thing rdonlyTabletStats := &discovery.TabletHealth{ Target: &querypb.Target{ TabletType: topodatapb.TabletType_RDONLY, }, } - // This call should not be forwarded to the go/vt/throttler.Throttler object. - throttler.state.StatsUpdate(rdonlyTabletStats) + // This call should not be forwarded to the go/vt/throttlerImpl.Throttler object. + throttlerImpl.state.StatsUpdate(rdonlyTabletStats) // The second throttle call should reject. - assert.True(t, throttler.Throttle(100)) - assert.Equal(t, int64(2), throttler.requestsTotal.Get()) - assert.Equal(t, int64(1), throttler.requestsThrottled.Get()) + assert.True(t, throttlerImpl.Throttle(100)) + assert.Equal(t, int64(2), throttlerImpl.requestsTotal.Get()) + assert.Equal(t, int64(1), throttlerImpl.requestsThrottled.Get()) // This call should not throttle due to priority. Check that's the case and counters agree. - assert.False(t, throttler.Throttle(0)) - assert.Equal(t, int64(3), throttler.requestsTotal.Get()) - assert.Equal(t, int64(1), throttler.requestsThrottled.Get()) - throttler.Close() - assert.Zero(t, throttler.throttlerRunning.Get()) + assert.False(t, throttlerImpl.Throttle(0)) + assert.Equal(t, int64(3), throttlerImpl.requestsTotal.Get()) + assert.Equal(t, int64(1), throttlerImpl.requestsThrottled.Get()) + throttlerImpl.Close() + assert.Zero(t, throttlerImpl.throttlerRunning.Get()) } func TestNewTxThrottler(t *testing.T) { @@ -155,28 +155,24 @@ func TestNewTxThrottler(t *testing.T) { env := tabletenv.NewEnv(config, t.Name()) { - // disabled config - throttler, err := newTxThrottler(env, nil, &txThrottlerConfig{enabled: false}) - assert.Nil(t, err) - assert.NotNil(t, throttler) - } - { - // enabled with invalid throttler config - throttler, err := newTxThrottler(env, nil, &txThrottlerConfig{ - enabled: true, - throttlerConfig: &throttlerdatapb.Configuration{}, - }) - assert.NotNil(t, err) - assert.Nil(t, throttler) + // disabled + config.EnableTxThrottler = false + throttler := NewTxThrottler(env, nil) + throttlerImpl, _ := throttler.(*txThrottler) + assert.NotNil(t, throttlerImpl) + assert.NotNil(t, throttlerImpl.config) + assert.False(t, throttlerImpl.config.enabled) } { // enabled - throttler, err := newTxThrottler(env, nil, &txThrottlerConfig{ - enabled: true, - healthCheckCells: []string{"cell1"}, - throttlerConfig: throttler.DefaultMaxReplicationLagModuleConfig().Configuration, - }) - assert.Nil(t, err) - assert.NotNil(t, throttler) + config.EnableTxThrottler = true + config.TxThrottlerHealthCheckCells = []string{"cell1", "cell2"} + config.TxThrottlerTabletTypes = &topoproto.TabletTypeListFlag{topodatapb.TabletType_REPLICA} + throttler := NewTxThrottler(env, nil) + throttlerImpl, _ := throttler.(*txThrottler) + assert.NotNil(t, throttlerImpl) + assert.NotNil(t, throttlerImpl.config) + assert.True(t, throttlerImpl.config.enabled) + assert.Equal(t, []string{"cell1", "cell2"}, throttlerImpl.config.healthCheckCells) } }