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

Feature Request: filter topology by shard in vttablet txthrottler #13068

Closed
timvaillancourt opened this issue May 10, 2023 · 5 comments
Closed

Comments

@timvaillancourt
Copy link
Contributor

Feature Description

The txthrottler (on the PRIMARY) uses healthcheck streams to determine the replication lag of replicas in the shard

Today, the txthrottler uses discovery.NewCellTabletsWatcher to watch the topology with a filter on the keyspace only. This means the topology watcher is "watching" more tablets than is necessary

This feature request is to support filtering by keyspace AND shard

Use Case(s)

Standard deploy w/TxThrottler enabled

@timvaillancourt timvaillancourt added Needs Triage This issue needs to be correctly labelled and triaged Type: Feature Request labels May 10, 2023
@frouioui frouioui added Component: Cluster management and removed Needs Triage This issue needs to be correctly labelled and triaged labels May 11, 2023
@frouioui
Copy link
Member

@vitessio/cluster-management

@harshit-gangal
Copy link
Member

Before was the health checks from other shards ignored? Were they taking part in determining that throttling to be done?

@timvaillancourt
Copy link
Contributor Author

timvaillancourt commented May 18, 2023

Before was the health checks from other shards ignored?

@harshit-gangal in v14 and below only the local shard was watched, which I feel is more correct/efficient

This behaviour was changed to filter by keyspace-only here in this PR from v15

Were they taking part in determining that throttling to be done?

I feel the Transaction Throttler (on PRIMARY) should only be concerned about replicas within it's local shard. This will prevent lag in one shard affecting all

@harshit-gangal
Copy link
Member

That looks like the side effect of removing the legacy health check. Thank you for digging into that.

@timvaillancourt
Copy link
Contributor Author

Indirectly resolved by #14412. Thanks @deepthi!

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