Skip to content

Commit

Permalink
slack-vitess-r15.0.5: backport Transaction Throttler PRs, pt. 1 (#335)
Browse files Browse the repository at this point in the history
* Add basic metrics to `vttablet` transaction throttler (vitessio#12418)

* Add basic stats to vttablet tx throttler

Signed-off-by: Tim Vaillancourt <[email protected]>

* test new metrics

Signed-off-by: Tim Vaillancourt <[email protected]>

* reorder

Signed-off-by: Tim Vaillancourt <[email protected]>

* short names

Signed-off-by: Tim Vaillancourt <[email protected]>

* Add max rate

Signed-off-by: Tim Vaillancourt <[email protected]>

* Move NewGaugeFunc to under conditional

Signed-off-by: Tim Vaillancourt <[email protected]>

* Use env

Signed-off-by: Tim Vaillancourt <[email protected]>

* Remove env from TxThrottler struct

Signed-off-by: Tim Vaillancourt <[email protected]>

* Fix tests

Signed-off-by: Tim Vaillancourt <[email protected]>

* PR suggestion

Signed-off-by: Tim Vaillancourt <[email protected]>

* Fix unit test

Signed-off-by: Tim Vaillancourt <[email protected]>

* reorder test vars

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>

* 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 <[email protected]>

* Add missing override of max replication lag in `throttler.newThrottler()`

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Reorder functions to make diff easier to read

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix check for maxRate in `newThrottlerFromConfig()`

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix some CI pipeline issues

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Address PR comment.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix typo

Signed-off-by: Eduardo J. Ortega U <[email protected]>

---------

Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Eduardo J. Ortega U. <[email protected]>

* Cleanup panics in `txthrottler`, reorder for readability (vitessio#12901)

* Cleanup tx_throttler.go

Signed-off-by: Tim Vaillancourt <[email protected]>

* Cleanup tx_throttler.go #2

Signed-off-by: Tim Vaillancourt <[email protected]>

* Fix throttlerFactoryFunc

Signed-off-by: Tim Vaillancourt <[email protected]>

* Undo if-cond consolidation

Signed-off-by: Tim Vaillancourt <[email protected]>

* Undo struct shuffling

Signed-off-by: Tim Vaillancourt <[email protected]>

* prove that disabled config returns nil error

Signed-off-by: Tim Vaillancourt <[email protected]>

* Improve test

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>

* 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 <[email protected]>

* make linter happy

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* fix flags e2e test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Fix flags e2e test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Address PR comments

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fixes

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix a comment

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix e2e flag test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Update JS code for protobuf changes.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix QueryEngine unit test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix e2e flag test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Fix spurious tab in comment

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Address PR comment

Don't use dual format flag for new flags - stick with - separated ones.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

---------

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* remove mistaken git add

Signed-off-by: Tim Vaillancourt <[email protected]>

* make vtadmin_web_proto_types

Signed-off-by: Tim Vaillancourt <[email protected]>

* test unit_race test on go-version: 1.18.9

Signed-off-by: Tim Vaillancourt <[email protected]>

* 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 <[email protected]>

* Fix setup order to avoid races (vitessio#13871)

Signed-off-by: Dirkjan Bussink <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Eduardo J. Ortega U. <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
Co-authored-by: Eduardo J. Ortega U <[email protected]>
Co-authored-by: Matt Lord <[email protected]>
Co-authored-by: Dirkjan Bussink <[email protected]>
  • Loading branch information
4 people authored May 10, 2024
1 parent 3c6f27b commit abadd5a
Show file tree
Hide file tree
Showing 45 changed files with 1,291 additions and 951 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cluster_endtoend_xb_backup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ jobs:
# install JUnit report formatter
go install github.com/vitessio/go-junit-report@HEAD
sudo apt-get install percona-xtrabackup-80 lz4
sudo apt-get install -y percona-xtrabackup-80 lz4
- name: Run cluster endtoend test
if: steps.skip-workflow.outputs.skip-workflow == 'false' && steps.changes.outputs.end_to_end == 'true'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/cluster_endtoend_xb_recovery.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ jobs:
# install JUnit report formatter
go install github.com/vitessio/go-junit-report@HEAD
sudo apt-get install percona-xtrabackup-80 lz4
sudo apt-get install -y percona-xtrabackup-80 lz4
- name: Run cluster endtoend test
if: steps.skip-workflow.outputs.skip-workflow == 'false' && steps.changes.outputs.end_to_end == 'true'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/codeql_analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
- name: Building binaries
timeout-minutes: 30
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/endtoend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,8 @@ jobs:
max_attempts: 3
retry_on: error
command: |
# We set the VTDATAROOT to the /tmp folder to reduce the file path of mysql.sock file
# which musn't be more than 107 characters long.
export VTDATAROOT="/tmp/"
eatmydata -- tools/e2e_test_runner.sh
15 changes: 12 additions & 3 deletions .github/workflows/unit_race.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ jobs:
- name: unit_race
if: steps.skip-workflow.outputs.skip-workflow == 'false' && steps.changes.outputs.unit_tests == 'true'
timeout-minutes: 30
run: |
eatmydata -- make unit_test_race
uses: nick-fields/retry@v2
with:
timeout_minutes: 30
max_attempts: 3
retry_on: error
command: |
# We set the VTDATAROOT to the /tmp folder to reduce the file path of mysql.sock file
# which musn't be more than 107 characters long.
export VTDATAROOT="/tmp/"
export NOVTADMINBUILD=1
eatmydata -- make unit_test_race
6 changes: 6 additions & 0 deletions .github/workflows/unit_test_mysql80.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,10 @@ jobs:
max_attempts: 3
retry_on: error
command: |
set -exo pipefail
# We set the VTDATAROOT to the /tmp folder to reduce the file path of mysql.sock file
# which musn't be more than 107 characters long.
export VTDATAROOT="/tmp/"
export NOVTADMINBUILD=1
eatmydata -- make unit_test
2 changes: 1 addition & 1 deletion .github/workflows/upgrade_downgrade_test_backups_e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Check out other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Check out other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Checkout to the other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Checkout to the other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Check out other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Check out other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Check out other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Check out other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Check out other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the next release of Vitess
- name: Check out other version's code (${{ needs.get_next_release.outputs.next_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Check out other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ jobs:
sudo apt-get install -y gnupg2
sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
sudo apt-get update
sudo apt-get install percona-xtrabackup-24
sudo apt-get install -y percona-xtrabackup-24
# Checkout to the last release of Vitess
- name: Check out other version's code (${{ needs.get_previous_release.outputs.previous_release }})
Expand Down
10 changes: 8 additions & 2 deletions go/cmd/vttestserver/vttestserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ func TestPersistentMode(t *testing.T) {
// reboot the persistent cluster
cluster.TearDown()
cluster, err = startPersistentCluster(dir)
defer cluster.TearDown()
defer func() {
cluster.PersistentMode = false // Cleanup the tmpdir as we're done
cluster.TearDown()
}()
assert.NoError(t, err)

// rerun our sanity checks to make sure vschema migrations are run during every startup
Expand Down Expand Up @@ -249,7 +252,10 @@ func TestMtlsAuth(t *testing.T) {
fmt.Sprintf("--vtctld_grpc_ca=%s", caCert),
fmt.Sprintf("--grpc_auth_mtls_allowed_substrings=%s", "CN=ClientApp"))
assert.NoError(t, err)
defer cluster.TearDown()
defer func() {
cluster.PersistentMode = false // Cleanup the tmpdir as we're done
cluster.TearDown()
}()

// startCluster will apply vschema migrations using vtctl grpc and the clientCert.
assertColumnVindex(t, cluster, columnVindex{keyspace: "test_keyspace", table: "test_table", vindex: "my_vdx", vindexType: "hash", column: "id"})
Expand Down
1 change: 1 addition & 0 deletions go/flags/endtoend/vttablet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Usage of vttablet:
--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.
Expand Down
2 changes: 1 addition & 1 deletion go/vt/grpcclient/client_flaky_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestDialErrors(t *testing.T) {
t.Fatal(err)
}
vtg := vtgateservicepb.NewVitessClient(gconn)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
_, err = vtg.Execute(ctx, &vtgatepb.ExecuteRequest{})
cancel()
gconn.Close()
Expand Down
Loading

0 comments on commit abadd5a

Please sign in to comment.