Skip to content
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

Bug Report: vttablet w/--enable-tx-throttler starts with disabled tx throttler #12410

Closed
timvaillancourt opened this issue Feb 21, 2023 · 1 comment · Fixed by #13115
Closed

Comments

@timvaillancourt
Copy link
Contributor

timvaillancourt commented Feb 21, 2023

Overview of the Issue

👋 Vitess,

I've noticed that vttablet will successfully start when the user has enabled the transaction throttler (using --enable-tx-throttler) and the throttler fails to start up.

Instead of failing to start as I expected, vttablet logs an error (below) and continues to appear healthy/operational.

Example due to missing, required initial_rate field in -tx-throttler-config:

E0220 09:43:58.985198    3187 tx_throttler.go:88] Error creating transaction throttler. Transaction throttling will be disabled. Error: initial_rate must be >= 1

NOTE: after this error vttablet continues to start as normal

I believe this behaviour was implemented intentionally, however I feel it is unsafe and confusing to the user that asked for something to be enabled that is nearly-silently disabled in reality. It took quite a while for me to notice this was the cause of my throttling problems, for example.

My preference is that vttablet fails to start in this scenario, which I feel would be a valid, but "breaking change" to the tx throttler 🤔

Assuming agreement this is a bug/problem, I am happy to contribute a fix for this myself 👍

Reproduction Steps

  1. Start vttablet with transaction throttler and an incomplete tx-throttler configuration
    • Example:
      vttablet -tablet-path us_east_1e-123456 \
        -mycnf-file /mnt/vitess/mysql/datadir/my.cnf -topo_implementation "consul" \
        -topo_global_server_address "consul.redacted.com:8500" -topo_global_root "vitess/global" \
        -init_keyspace test -init_shard -80 -init_tablet_type "replica" \
        -enable-tx-throttler 
      
    • Notice the required tx-throttler flag --tx-throttler-healthcheck-cells is not specified
  2. Noticed that Error creating transaction throttler. Transaction throttling will be disabled. is logged, but vttablet continues to startup

Full example

$ vttablet -tablet-path us_east_1e-123456 \
    -mycnf-file /etc/my.cnf -topo_implementation "consul" \
    -topo_global_server_address "consul.redacted.com:8500" -topo_global_root "vitess/global" \
    -init_keyspace test -init_shard -80 -init_tablet_type "replica" \
    -enable-tx-throttler 
E0221 10:13:18.843054    6307 tx_throttler.go:88] Error creating transaction throttler. Transaction throttling will be disabled. Error: empty healthCheckCells given. &{enabled:true topoServer:0xc0012319c0 throttlerConfig:0xc001217950 healthCheckCells:[]}
W0221 10:13:18.970389    6307 tm_init.go:706] creating keyspace and shard failed (Get "http://consul.redacted.com:8500/v1/kv/vitess/global/keyspaces/test/shards/-80/Shard": dial tcp: lookup consul.redacted.com on 127.0.0.1:53: no such host), backing off 1s before retrying
W0221 10:13:19.973236    6307 tm_init.go:706] creating keyspace and shard failed (Get "http://consul.redacted.com:8500/v1/kv/vitess/global/keyspaces/test/shards/-80/Shard": dial tcp: lookup consul.redacted.com on 127.0.0.1:53: no such host), backing off 1.125602181s before retrying
W0221 10:13:21.101325    6307 tm_init.go:706] creating keyspace and shard failed (Get "http://consul.redacted.com:8500/v1/kv/vitess/global/keyspaces/test/shards/-80/Shard": dial tcp: lookup consul.redacted.com on 127.0.0.1:53: no such host), backing off 1.295018994s before retrying
^C

Binary Version

vttablet binary built from slackhq/vitess (fork) based on vitessio/vitess tag: v12.0.5

Our fork has no changes to go/vt/vttablet/tabletserver/txthrottler code

I expect this issue exists on v12+ as well

Operating System and Environment details

- Ubuntu 18.04.6 LTS
- Linux 5.4.0-1096-aws
- x86_64

Log Fragments

E0220 09:43:58.985198    3187 tx_throttler.go:88] Error creating transaction throttler. Transaction throttling will be disabled. Error: initial_rate must be >= 1
@timvaillancourt timvaillancourt added Needs Triage This issue needs to be correctly labelled and triaged Type: Bug labels Feb 21, 2023
@timvaillancourt timvaillancourt changed the title Bug Report: vttablet w/--enable-tx-throttler will start with disabled tx throttler Bug Report: vttablet w/--enable-tx-throttler starts with disabled tx throttler Feb 21, 2023
@harshit-gangal harshit-gangal removed the Needs Triage This issue needs to be correctly labelled and triaged label Feb 23, 2023
@shlomi-noach
Copy link
Contributor

I agree this is a bug. The correct solution is to have a sane default for cells, and the default would be "all cells". And I see you've done exactly that in #12435, which I'll review next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants