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

Throttler multi-metrics: proto changes #92

Conversation

shlomi-noach
Copy link

@shlomi-noach shlomi-noach commented May 26, 2024

Replaced by vitessio#16040

Description

This PR is the first in the series of incremental changes breaking down vitessio#15988 into smaller parts.

We begin by applying the proto changes.

Notable:

  • The CheckThrottler gRPC call already exists in v19 and before, and is/was used by the Primary throttler to probe its replicas, and by the replica throttler to notify the Primary about recent checks.
  • Here, we enhance CheckThrottlerRequest and CheckThrottlerResponse with more fields. We will later make CheckThrottler a vtctidlclient command. A check requests identifies by app name, and now also clearly indicates whether multi-metrics are supported/desired (important for backwards compatibility and cross-version interaction), as well as other flags that will be come clearer later on.
  • GetThrottlerStatus, GetThrottlerStatusRequest, GetThrottlerStatusResponse are new calls, and useful for debugging/incident analysis. These return the equivalent of /throttler/status, but now via gRPC instead of HTTP. Likewise, this will be implemented as a vtctldclient in a followup PR.
  • ThrottlerConfig now supports:
    • AppCheckedMetrics - mapping of app->metrics (meaning which metrics the throttler checks on behalf of given app)
    • MetricThresholds - per metric threshold (different threshold for lag, for threads_running, etc.)

See the docs in vitessio#15988 main comment for more.

This PR merges into planetscale:throttler-multi-metrics-incremental, which is a Draft PR, and not into main.

https://github.com/planetscale/vitess/compare/planetscale:vitess:throttler-multi-metrics-incremental...planetscale:vitess:throttler-multi-metrics-incremental-proto?expand=1

Related Issue(s)

vitessio#15988

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach requested a review from a team May 26, 2024 05:53
@shlomi-noach shlomi-noach changed the base branch from throttler-multi-metrics-incremental to main June 2, 2024 14:28
@shlomi-noach shlomi-noach changed the base branch from main to throttler-multi-metrics-incremental June 2, 2024 14:29
@shlomi-noach
Copy link
Author

Replaced by vitessio#16040, whose base is vitessio:vitess:throttler-multi-metrics-incremental.

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

Successfully merging this pull request may close these issues.

1 participant