-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
SHOW VITESS_REPLICATION_STATUS: Only use replication tracker when it's enabled #15348
Conversation
Signed-off-by: Matt Lord <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Matt Lord <[email protected]>
96a506d
to
5076415
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15348 +/- ##
==========================================
- Coverage 67.53% 65.43% -2.11%
==========================================
Files 1561 1561
Lines 193387 193531 +144
==========================================
- Hits 130607 126634 -3973
- Misses 62780 66897 +4117 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
This reverts commit 6d07ac2. CheckThrottler is a tmclient RPC can thus can't be called from a non-tablet. Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
As check?app=vtgate is only valid on a PRIMARY. Also, vtgate's don't interface directly with the tablet throttler so it doesn't make sense to use the vtgate app name. Signed-off-by: Matt Lord <[email protected]>
769c764
to
266eccb
Compare
Signed-off-by: Matt Lord <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
Signed-off-by: Matt Lord <[email protected]>
…s enabled (#15348) Signed-off-by: Matt Lord <[email protected]>
…s enabled (#15348) Signed-off-by: Matt Lord <[email protected]>
…racker when it's enabled (#15348) (#15361) Signed-off-by: Matt Lord <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Matt Lord <[email protected]>
…racker when it's enabled (#15348) (#15360) Signed-off-by: Matt Lord <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Matt Lord <[email protected]>
…racker when it's enabled (#15348) (#15362) Signed-off-by: Matt Lord <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Matt Lord <[email protected]>
Description
We added the
show vitess_replication_status
query in #8900. At the time I chose to use the tablet's replication lag stats variable as we could get a more accurate representation of the lag than using mysqld'sseconds_behind_[master|source]
value. The stat relied on polling — in which case we can also estimate the lag when replication is not running – or sub-second heartbeats, and it was more accurate for these reasons.This query was first introduced in v12 and at that time the
ReplicationTracker
was always enabled in the local examples using the polling method, where the manual testing was done. In v17 we changed the vttablet flags in the local examples in order to switch from the polling based method to the heartbeat based method before subsequently removing the heartbeats as well and after that we instead started using on-demand heartbeats alone — so today the replication tracker is disabled entirely in the local examples.In any event, the real issue was that we didn't have adequate testing of this feature. This PR addresses the issue by:
You can see a manual test case here: #15341 (comment)
I think that this should be backported for the following reasons:
I also updated the throttler check to use
check-self
ascheck?app=vtgate
is not useful because:check
is only applicable on aPRIMARY
tablet and we are checking non-PRIMARY
tabletsvtgate
doesn't make sense asvtgate
does not interface with the tablet throttlerRelated Issue(s)
Checklist