-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add basic metrics to vttablet
transaction throttler
#12418
Add basic metrics to vttablet
transaction throttler
#12418
Conversation
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Also check if anything needs to be updated here https://vitess.io/docs/17.0/reference/features/tablet-throttler/ |
@harshit-gangal I believe the Transaction (tx) Throttler has no website documentation The link you provided documents the "Tablet Throttler" in this code location which is a similar but different throttler Assuming our POCs are successful and we understand the internals enough to document it (we don't right now - the "rate" logic |
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
* Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
* Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
…#67) * Emit per workload labels for existing per table vttablet metrics (vitessio#12394) * Emit per workload labels for existing per table vttablet metrics This adds the possibility to configure vttablet (via CLI flag) to also have a workload label for existing per table metrics (query counts, query times, query errors, query rows affected, query rows returned, query error counts). Workload can be any string that makes sense for the client application. For example, API endpoint name, controller, batch job name, application name or something else. This is usefult to be able to gain observability about how the query load is distributed across different workloads. This is achieved with two new CLI flags, namely: * `enable-per-workload-table-metrics`: whether to enable or disable per workload metric collection - disabled by default to preserve the current behavior, thus making the new feature opt-in only. * `workload-label`: a string to look for in query comments to identify the workload running the current query. The workload is obtained by parsing query comments of the form: /* ... <workload_label>=<workload_name>; ... */ For example, if vttablet is started with `--enable-per-workload-table-metrics --workload-label app_name` anda query is issued with a comment like /* ... app_name=shop; ... */ then metrics will look like ``` vttablet_query_counts{plan="Select",table="dual", workload="shop"} 15479 ``` instead of ``` vttablet_query_counts{plan="Select",table="dual"} 15479 ``` Query comment parsing only takes place if `--enable-per-workload-table-metrics` is used, as to not incur parsing performance impact if the user does not want per workload metrics. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * make linter happy Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * fix flags e2e test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comments: * Obtain workload information on the vtgate instead of the vttablet, avoiding double parsing. * Treat workload name as a query directive. * Send workload name from vtgate to vttablet as ExecuteOptions. Additionally, annotate tabletserver's execution span with the workload name to also enrich traces with workload name data, in addition to metrics. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * A few fixes: 1. Rebuild some files with `make proto`. 2. Protect against nil ExecuteOptions on the tabletserver. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix flags e2e test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comments Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fixes Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix a comment Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix e2e flag test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Update JS code for protobuf changes. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix QueryEngine unit test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix e2e flag test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix spurious tab in comment Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comment Don't use dual format flag for new flags - stick with - separated ones. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix cherry-pick * Add basic metrics to `vttablet` transaction throttler (vitessio#12418) * Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix go/vt/vttablet/tabletserver/query_engine_test.go --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
* Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
* Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
* Add basic metrics to `vttablet` transaction throttler (vitessio#12418) * Add basic stats to vttablet tx throttler Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test new metrics Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * short names Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add max rate Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move NewGaugeFunc to under conditional Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use env Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Remove env from TxThrottler struct Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix tests Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * PR suggestion Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix unit test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * reorder test vars Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix transaction throttler ignoring the initial rate (vitessio#12618) * Fix transaction throttler ignoring the initial rate This addresses the issue reported in vitessio#12549 Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Add missing override of max replication lag in `throttler.newThrottler()` Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Reorder functions to make diff easier to read Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix check for maxRate in `newThrottlerFromConfig()` Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix some CI pipeline issues Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comment. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix typo Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com> * Cleanup panics in `txthrottler`, reorder for readability (vitessio#12901) * Cleanup tx_throttler.go Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Cleanup tx_throttler.go #2 Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Fix throttlerFactoryFunc Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Undo if-cond consolidation Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Undo struct shuffling Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * prove that disabled config returns nil error Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Improve test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Emit per workload labels for existing per table vttablet metrics (vitessio#12394) * Emit per workload labels for existing per table vttablet metrics This adds the possibility to configure vttablet (via CLI flag) to also have a workload label for existing per table metrics (query counts, query times, query errors, query rows affected, query rows returned, query error counts). Workload can be any string that makes sense for the client application. For example, API endpoint name, controller, batch job name, application name or something else. This is usefult to be able to gain observability about how the query load is distributed across different workloads. This is achieved with two new CLI flags, namely: * `enable-per-workload-table-metrics`: whether to enable or disable per workload metric collection - disabled by default to preserve the current behavior, thus making the new feature opt-in only. * `workload-label`: a string to look for in query comments to identify the workload running the current query. The workload is obtained by parsing query comments of the form: /* ... <workload_label>=<workload_name>; ... */ For example, if vttablet is started with `--enable-per-workload-table-metrics --workload-label app_name` anda query is issued with a comment like /* ... app_name=shop; ... */ then metrics will look like ``` vttablet_query_counts{plan="Select",table="dual", workload="shop"} 15479 ``` instead of ``` vttablet_query_counts{plan="Select",table="dual"} 15479 ``` Query comment parsing only takes place if `--enable-per-workload-table-metrics` is used, as to not incur parsing performance impact if the user does not want per workload metrics. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * make linter happy Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * fix flags e2e test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comments: * Obtain workload information on the vtgate instead of the vttablet, avoiding double parsing. * Treat workload name as a query directive. * Send workload name from vtgate to vttablet as ExecuteOptions. Additionally, annotate tabletserver's execution span with the workload name to also enrich traces with workload name data, in addition to metrics. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * A few fixes: 1. Rebuild some files with `make proto`. 2. Protect against nil ExecuteOptions on the tabletserver. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix flags e2e test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comments Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fixes Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix a comment Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix e2e flag test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Update JS code for protobuf changes. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix QueryEngine unit test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix e2e flag test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix spurious tab in comment Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comment Don't use dual format flag for new flags - stick with - separated ones. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * remove mistaken git add Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * make vtadmin_web_proto_types Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * test unit_race test on go-version: 1.18.9 Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert "test unit_race test on go-version: 1.18.9" This reverts commit 922e897. * CI: Misc test improvements to limit failures with various runners (vitessio#13825) Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix setup order to avoid races (vitessio#13871) Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com>
Description
This PR adds the metrics below to
vttablet
to make it easier to observe the transaction throttlerTxThrottlerRunning
0
(disabled)1
(enabled)TxThrottlerRequests
- total # of calls to.Throttle()
TxThrottlerThrottled
- total # of throttled calls to.Throttle()
Gauge:TxThrottlerMaxRate
- the max rate reported from the underlying throttler.MaxRate()
While updating unit tests for the transaction throttler, I moved the testing to use
github.com/stretchr/testify/assert
Related Issue(s)
Resolves: #12417
Checklist
Deployment Notes