Skip to content

Vitess: include cells filter in Vtctld API call #137

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

Merged
merged 5 commits into from
Nov 30, 2020

Conversation

timvaillancourt
Copy link
Contributor

@timvaillancourt timvaillancourt commented Nov 26, 2020

This PR causes Freno to add a Vitess-cell filter in the call it does to the vtctld API to gather tablets. Freno already supports filtering of tablets by cell but this currently happens client-side only. Filtering at the vtctld API should reduce the overhead of vtctld gathering tablet information (across many cells), the size of the response, etc

Support for filtering by cell(s) was added to the Vitess vtctld API used by Freno in vitessio/vitess#6569 (Vitess release 8.0.0+)

This change is backward-compatible as older Vitess releases will just-ignore the ?cells= query parameter and we're still filtering by cell client-side (will be deprecated in the future) - so both sides are always filtered 👍

Example of the cells filter:

$ curl -s https://<vtctld api>/api/keyspace/test_ks/tablets/ | jq length
7
$ curl -s https://<vtctld api>/api/keyspace/test_ks/tablets/?cells=cell1 | jq length
2
$ curl -s https://<vtctld api>/api/keyspace/test_ks/tablets/?cells=cell2 | jq length
2

@timvaillancourt timvaillancourt added this to the v1.1.1 milestone Nov 26, 2020
@timvaillancourt timvaillancourt temporarily deployed to staging November 27, 2020 19:52 Inactive
Copy link

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to construct query params using a URL encoder but this should work.

@timvaillancourt
Copy link
Contributor Author

It's better to construct query params using a URL encoder but this should work.

@shlomi-noach that's a good idea 👍. Updated to use net/url for building that URL

Copy link

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@timvaillancourt timvaillancourt merged commit a028184 into master Nov 30, 2020
@timvaillancourt timvaillancourt deleted the vitess-filter-cells-on-vtctld branch November 30, 2020 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants