[release-16.0] Throttler: Expose Tablet's Config & Leverage to Deflak…

…e Tests (#12791) * Throttler: Expose Tablet's Config & Leverage to Deflake Tests (#12737) * Flakes: effectively disable vtorc for deterministic behavior For example, we stop replication, wait a few seconds, then expect there to be lag. But vtorc could repair replication during that wait and then the lag is gone. Signed-off-by: Matt Lord <mattalord@gmail.com> * Wait for the throttler to be up and running everywhere Signed-off-by: Matt Lord <mattalord@gmail.com> * Expose tablet's throttler config and leverage to deflake tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Apply various corrections Signed-off-by: Matt Lord <mattalord@gmail.com> * Be more explicit about VTOrc behavior changes Signed-off-by: Matt Lord <mattalord@gmail.com> * Note received throttler response when it is unexpected Signed-off-by: Matt Lord <mattalord@gmail.com> * Fixes from local testing Signed-off-by: Matt Lord <mattalord@gmail.com> * Nits from self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Use assert.Equalf on failed assertions Signed-off-by: Matt Lord <mattalord@gmail.com> * Ummm, duh. Signed-off-by: Matt Lord <mattalord@gmail.com> * Try to get rid of last bit of flakiness Which seemed to revolve around NOT sleeping long enough after starting all the sleep queries. Signed-off-by: Matt Lord <mattalord@gmail.com> * Nits from self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Address review comments Signed-off-by: Matt Lord <mattalord@gmail.com> * Adjust test for behavior and comment it And adjust timing Signed-off-by: Matt Lord <mattalord@gmail.com> * Align both stale hearbeat checks Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove no longer needed flag This is because enabling heartbeats with --heartbeat_enable also results in the replication reporter being enabled: https://github.com/vitessio/vitess/blob/3d9ef871e42bd20a60ec95997c97ecf0694c1e78/go/vt/vttablet/tabletserver/tabletenv/config.go#L235-L237 Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct comment Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct comment part II: electric boogaloo Signed-off-by: Matt Lord <mattalord@gmail.com> * Revert one other minor unnecessary change. Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> * Post cherry-pick fixup Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com>
vitessio · Mar 31, 2023 · f2c1c13 · f2c1c13
1 parent 984793f
commit f2c1c13
Show file tree

Hide file tree

Showing 7 changed files with 334 additions and 122 deletions.
diff --git a/examples/common/scripts/vttablet-up.sh b/examples/common/scripts/vttablet-up.sh
@@ -46,7 +46,6 @@ vttablet \
  --init_shard $shard \
  --init_tablet_type $tablet_type \
  --health_check_interval 5s \
- --enable_replication_reporter \
  --backup_storage_implementation file \
  --file_backup_storage_root $VTDATAROOT/backups \
  --restore_from_backup \
@@ -56,6 +55,7 @@ vttablet \
  --pid_file $VTDATAROOT/$tablet_dir/vttablet.pid \
  --vtctld_addr http://$hostname:$vtctld_web_port/ \
  --disable_active_reparents \
+ --throttler-config-via-topo --heartbeat_enable --heartbeat_interval=250ms --heartbeat_on_demand_duration=5s \
  > $VTDATAROOT/$tablet_dir/vttablet.out 2>&1 &
 
 # Block waiting for the tablet to be listening

diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt
@@ -300,7 +300,7 @@ Usage of vttablet:
  --tablet_protocol string Protocol to use to make queryservice RPCs to vttablets. (default "grpc")
  --throttle_check_as_check_self Should throttler/check return a throttler/check-self result (changes throttler behavior for writes)
  --throttle_metrics_query SELECT Override default heartbeat/lag metric. Use either SELECT (must return single row, single value) or `SHOW GLOBAL ... LIKE ...` queries. Set -throttle_metrics_threshold respectively.
- --throttle_metrics_threshold float Override default throttle threshold, respective to -throttle_metrics_query (default 1.7976931348623157e+308)
+ --throttle_metrics_threshold float Override default throttle threshold, respective to --throttle_metrics_query (default 1.7976931348623157e+308)
  --throttle_tablet_types string Comma separated VTTablet types to be considered by the throttler. default: 'replica'. example: 'replica,rdonly'. 'replica' aways implicitly included (default "replica")
  --throttle_threshold duration Replication lag threshold for default lag throttling (default 1s)
  --throttler-config-via-topo When 'true', read config from topo service and ignore throttle_threshold, throttle_metrics_threshold, throttle_metrics_query, throttle_check_as_check_self

diff --git a/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go b/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go
@@ -30,6 +30,7 @@ import (
  "vitess.io/vitess/go/mysql"
  "vitess.io/vitess/go/test/endtoend/cluster"
  "vitess.io/vitess/go/test/endtoend/onlineddl"
+ "vitess.io/vitess/go/test/endtoend/throttler"
  "vitess.io/vitess/go/vt/schema"
  "vitess.io/vitess/go/vt/vttablet/tabletmanager/vreplication"
  throttlebase "vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/base"
@@ -259,13 +260,13 @@ func TestSchemaChange(t *testing.T) {
  err := clusterInstance.WaitForTabletsToHealthyInVtgate()
  require.NoError(t, err)
 
- _, err = onlineddl.UpdateThrottlerTopoConfig(clusterInstance, true, false, 0, "", false)
+ _, err = throttler.UpdateThrottlerTopoConfig(clusterInstance, true, false, 0, "", false)
  require.NoError(t, err)
 
  for _, ks := range clusterInstance.Keyspaces {
  for _, shard := range ks.Shards {
  for _, tablet := range shard.Vttablets {
- onlineddl.WaitForThrottlerStatusEnabled(t, tablet, extendedMigrationWait)
+ throttler.WaitForThrottlerStatusEnabled(t, tablet, true, nil, extendedMigrationWait)
  }
  }
  }

diff --git a/go/test/endtoend/onlineddl/vtctlutil.go b/go/test/endtoend/onlineddl/vtctlutil.go
@@ -17,8 +17,6 @@ limitations under the License.
 package onlineddl
 
 import (
- "context"
- "fmt"
  "testing"
  "time"
 
@@ -36,51 +34,3 @@ func CheckCancelAllMigrationsViaVtctl(t *testing.T, vtctlclient *cluster.VtctlCl
  _, err := vtctlclient.ApplySchemaWithOutput(keyspace, cancelQuery, cluster.VtctlClientParams{SkipPreflight: true})
  assert.NoError(t, err)
 }
-
-// UpdateThrottlerTopoConfig runs vtctlclient UpdateThrottlerConfig.
-// This retries the command until it succeeds or times out as the
-// SrvKeyspace record may not yet exist for a newly created
-// Keyspace that is still initializing before it becomes serving.
-func UpdateThrottlerTopoConfig(clusterInstance *cluster.LocalProcessCluster, enable bool, disable bool, threshold float64, metricsQuery string, viaVtctldClient bool) (result string, err error) {
- args := []string{}
- clientfunc := clusterInstance.VtctldClientProcess.ExecuteCommandWithOutput
- if !viaVtctldClient {
- args = append(args, "--")
- clientfunc = clusterInstance.VtctlclientProcess.ExecuteCommandWithOutput
- }
- args = append(args, "UpdateThrottlerConfig")
- if enable {
- args = append(args, "--enable")
- }
- if disable {
- args = append(args, "--disable")
- }
- if threshold > 0 {
- args = append(args, "--threshold", fmt.Sprintf("%f", threshold))
- }
- if metricsQuery != "" {
- args = append(args, "--custom-query", metricsQuery)
- args = append(args, "--check-as-check-self")
- } else {
- args = append(args, "--check-as-check-shard")
- }
- args = append(args, clusterInstance.Keyspaces[0].Name)
-
- ctx, cancel := context.WithTimeout(context.Background(), throttlerConfigTimeout)
- defer cancel()
-
- ticker := time.NewTicker(time.Second)
- defer ticker.Stop()
-
- for {
- result, err = clientfunc(args...)
- if err == nil {
- return result, nil
- }
- select {
- case <-ctx.Done():
- return "", fmt.Errorf("timed out waiting for UpdateThrottlerConfig to succeed after %v. Last seen value: %+v, error: %v", throttlerConfigTimeout, result, err)
- case <-ticker.C:
- }
- }
-}