Deprecating and removing tablet throttler CLI flags and tests (#13246)

* Table throttler: --throttler-config-via-topo now defaults to 'true' Signed-off-by: Shlomi Noach <[email protected]> * add deprecation message Signed-off-by: Shlomi Noach <[email protected]> * endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere Signed-off-by: Shlomi Noach <[email protected]> * always use vtctldclient Signed-off-by: Shlomi Noach <[email protected]> * use cluster.VtctldClientProcess Signed-off-by: Shlomi Noach <[email protected]> * disable --throttler-config-via-topo in old throttler tests Signed-off-by: Shlomi Noach <[email protected]> * Remove --throttler-config-via-topo where used, since it now defaults 'true' Signed-off-by: Shlomi Noach <[email protected]> * fix vreplication cluster setup, waiting for throttler config to apply Signed-off-by: Shlomi Noach <[email protected]> * changelog Signed-off-by: Shlomi Noach <[email protected]> * extend throttler threshold Signed-off-by: Shlomi Noach <[email protected]> * a bit more verbose Signed-off-by: Shlomi Noach <[email protected]> * fixed CLI test Signed-off-by: Shlomi Noach <[email protected]> * remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration' Signed-off-by: Shlomi Noach <[email protected]> * more log info in throttler.Open() Signed-off-by: Shlomi Noach <[email protected]> * more logging Signed-off-by: Shlomi Noach <[email protected]> * Revert to --heartbeat_enable Signed-off-by: Shlomi Noach <[email protected]> * Protect throttler config change application with initMutex And in e2e test update the throttler config on the keyspace when it's created. Only wait for the new tablets in a shard to have the throttler enabled when adding a Shard. Signed-off-by: Matt Lord <[email protected]> * More CI testing Signed-off-by: Matt Lord <[email protected]> * CI testing cont Signed-off-by: Matt Lord <[email protected]> * Yes... Signed-off-by: Matt Lord <[email protected]> * Somebody doesn't like force pushes so msg here Signed-off-by: Matt Lord <[email protected]> * Increase on-demand heartbeat duration from 10s to 1m Signed-off-by: Matt Lord <[email protected]> * Use only on-demand heartbeats everywhere Signed-off-by: Matt Lord <[email protected]> * Use same throttler config everywhere Signed-off-by: Matt Lord <[email protected]> * Update all keyspaces and don't fail test on missing JSON keys Signed-off-by: Matt Lord <[email protected]> * Use constant heartbeats in vrepl e2e tests Until #13175 is fixed. Signed-off-by: Matt Lord <[email protected]> * Increase workflow command timeout Signed-off-by: Matt Lord <[email protected]> * Don't wait for throttler on non-serving primaries Signed-off-by: Matt Lord <[email protected]> * #13175 is fixed, therefore re-instating on-deman heartbeats Signed-off-by: Shlomi Noach <[email protected]> * Added ToC Signed-off-by: Shlomi Noach <[email protected]> * Tweak comment and kick CI Signed-off-by: Matt Lord <[email protected]> * Treat isOpen as the ready/running signal. Also align all initMutex usage. Signed-off-by: Matt Lord <[email protected]> * Re-adjust comment Signed-off-by: Matt Lord <[email protected]> * Adjust CheckIsReady() to match OnlineDDL's expectation/usage This was only using IsReady() before, now it's using IsOpen() and IsReady(). Signed-off-by: Matt Lord <[email protected]> * Get rid of log messages from SrvKeyspaceWatcher when no node/key Signed-off-by: Matt Lord <[email protected]> * More corrections/tweaks Signed-off-by: Matt Lord <[email protected]> * Use more convenient/clear new IsRunning function Signed-off-by: Matt Lord <[email protected]> * Revert "Use more convenient/clear new IsRunning function" This reverts commit 9aef276 as this change was not correct. Signed-off-by: Matt Lord <[email protected]> * Further fix correct use of IsOpen(), IsRunning(), IsEnabled() Signed-off-by: Shlomi Noach <[email protected]> * throttler.throttledApps cannot be nil Signed-off-by: Shlomi Noach <[email protected]> * Remove --enable_lag_throttler flag Signed-off-by: Shlomi Noach <[email protected]> * Deprecate --throttler_config_via_topo Signed-off-by: Shlomi Noach <[email protected]> * remove throttler mitigation code, as the problem was solved in #13195 Signed-off-by: Shlomi Noach <[email protected]> * deperecate throttler config flags Signed-off-by: Shlomi Noach <[email protected]> * Removed tabletmanager_throttler and tabletmanager_throttler_custom_config tests Signed-off-by: Shlomi Noach <[email protected]> * changelog Signed-off-by: Shlomi Noach <[email protected]> * remove EnableThrottler() call Signed-off-by: Shlomi Noach <[email protected]> * restore default value Signed-off-by: Shlomi Noach <[email protected]> * update threshold Signed-off-by: Shlomi Noach <[email protected]> * update flags desc Signed-off-by: Shlomi Noach <[email protected]> * using atomic.Bool Signed-off-by: Shlomi Noach <[email protected]> * Update changelog/18.0/18.0.0/summary.md Co-authored-by: Matt Lord <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> * use MarkDeprecated Signed-off-by: Shlomi Noach <[email protected]> * do not expect flags in vttablet --help Signed-off-by: Shlomi Noach <[email protected]> * remove --throttler-config-via-topo from examples scripts Signed-off-by: Shlomi Noach <[email protected]> --------- Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Matt Lord <[email protected]> Co-authored-by: Matt Lord <[email protected]>
vitessio · Jul 13, 2023 · c71c26a · c71c26a
1 parent a960b91
commit c71c26a
Show file tree

Hide file tree

Showing 17 changed files with 67 additions and 1,003 deletions.
diff --git a/.github/workflows/cluster_endtoend_tabletmanager_throttler.yml b/.github/workflows/cluster_endtoend_tabletmanager_throttler.yml
diff --git a/.github/workflows/cluster_endtoend_tabletmanager_throttler_custom_config.yml b/.github/workflows/cluster_endtoend_tabletmanager_throttler_custom_config.yml
diff --git a/changelog/18.0/18.0.0/summary.md b/changelog/18.0/18.0.0/summary.md
@@ -9,6 +9,7 @@
   - **[VTAdmin](#vtadmin)**
     - [Updated to node v18.16.0](#update-node)
   - **[Deprecations and Deletions](#deprecations-and-deletions)**
+    - [Deprecated Flags](#deprecated-flags)
     - [Deleted `k8stopo`](#deleted-k8stopo)
     - [Deleted `vtgr`](#deleted-vtgr)
   - **[New stats](#new-stats)**
@@ -39,6 +40,17 @@ here https://nodejs.org/en/blog/release/v18.16.0.
 
 ### <a id="deprecations-and-deletions"/>Deprecations and Deletions
 
+#### <a id="deprecated-flags"/>Deprecated Command Line Flags
+
+Throttler related `vttablet` flags:
+
+- `--enable-lag-throttler` is now removed after being deprecated in `v17.0`
+- `--throttle_threshold` is deprecated and will be removed in `v19.0`
+- `--throttle_metrics_query` is deprecated and will be removed in `v19.0`
+- `--throttle_metrics_threshold` is deprecated and will be removed in `v19.0`
+- `--throttle_check_as_check_self` is deprecated and will be removed in `v19.0`
+- `--throttler-config-via-topo` is deprecated after asummed `true` in `v17.0`. It will be removed in a future version.
+
 #### <a id="deleted-k8stopo"/>Deleted `k8stopo`
 
 The `k8stopo` has been deprecated in Vitess 17, also see https://github.com/vitessio/vitess/issues/13298. With Vitess 18

diff --git a/examples/common/scripts/vttablet-up.sh b/examples/common/scripts/vttablet-up.sh
@@ -54,7 +54,10 @@ vttablet \
  --service_map 'grpc-queryservice,grpc-tabletmanager,grpc-updatestream' \
  --pid_file $VTDATAROOT/$tablet_dir/vttablet.pid \
  --vtctld_addr http://$hostname:$vtctld_web_port/ \
- --throttler-config-via-topo --heartbeat_enable --heartbeat_interval=250ms --heartbeat_on_demand_duration=5s \
+ --disable_active_reparents \
+ --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
@@ -99,14 +99,12 @@ Usage of vttablet:
       --emit_stats                                                       If set, emit stats to push-based monitoring and stats backends
       --enable-consolidator                                              Synonym to -enable_consolidator (default true)
       --enable-consolidator-replicas                                     Synonym to -enable_consolidator_replicas
-      --enable-lag-throttler                                             Synonym to -enable_lag_throttler
       --enable-per-workload-table-metrics                                If true, query counts and query error metrics include a label that identifies the workload
       --enable-tx-throttler                                              Synonym to -enable_tx_throttler
       --enable_consolidator                                              This option enables the query consolidator. (default true)
       --enable_consolidator_replicas                                     This option enables the query consolidator only on replicas.
       --enable_hot_row_protection                                        If true, incoming transactions for the same row (range) will be queued and cannot consume all txpool slots.
       --enable_hot_row_protection_dry_run                                If true, hot row protection is not enforced but logs if transactions would have been queued.
-      --enable_lag_throttler                                             If true, vttablet will run a throttler service, and will implicitly enable heartbeats
       --enable_replication_reporter                                      Use polling to track replication lag.
       --enable_transaction_limit                                         If true, limit on number of transactions open at the same time will be enforced for all users. User trying to open a new transaction after exhausting their limit will receive an error immediately, regardless of whether there are available slots or not.
       --enable_transaction_limit_dry_run                                 If true, limit on number of transactions open at the same time will be tracked for all users, but not enforced.
@@ -313,12 +311,7 @@ Usage of vttablet:
       --tablet_manager_grpc_server_name string                           the server name to use to validate server certificate
       --tablet_manager_protocol string                                   Protocol to use to make tabletmanager RPCs to vttablets. (default "grpc")
       --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_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 (default true)
       --topo_consul_lock_delay duration                                  LockDelay for consul session. (default 15s)
       --topo_consul_lock_session_checks string                           List of checks for consul session. (default "serfHealth")
       --topo_consul_lock_session_ttl string                              TTL for consul session.