Fallback to poller replication lag if heartbeat lag fails #207
Merged
ejortegau merged 5 commits intoslack-vitess-r14.0.5from Mar 27, 2024
Merged
Conversation
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
| // rt.mode == tabletenv.Poller or fallback after heartbeat error | ||
| mysqlLag, mysqlErr = rt.poller.Status() | ||
| if fallbackToPoller && mysqlErr != nil { | ||
| return 0, errFallback |
There was a problem hiding this comment.
@ejortegau what if we just return rt.poller.Status() here? This would give you a more useful error although it wouldn't be clear there was a "fallback"
Wrapping the mysqlErr with errFallback might work too
Author
There was a problem hiding this comment.
I want to explicitly return a different error when fallback fails. I will do some wrapping, though
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
timvaillancourt
approved these changes
Feb 14, 2024
timvaillancourt
left a comment
timvaillancourt
left a comment
There was a problem hiding this comment.
I suggested one typo-fix in the error, but LGTM 👍
pbibra
reviewed
Mar 13, 2024
| if heartbeatLag, heartbeatErr = rt.hr.Status(); heartbeatErr == nil { | ||
| return heartbeatLag, heartbeatErr | ||
| } | ||
| fallbackToPoller = true |
There was a problem hiding this comment.
do we need this boolean? If either of the case statements are met, we would return out anyways
There was a problem hiding this comment.
then the condition below on line 158 can just be
if mysqlErr != nil {...}
Author
There was a problem hiding this comment.
I think so. The intention is to raise different errors in case rt.poller.Status() failed depending on whether fallback was used or not.
pbibra
approved these changes
Mar 14, 2024
…beat_poller_replication_tracker
ejortegau
deleted the
ejortegau/simultaneous_heartbeat_poller_replication_tracker
branch
timvaillancourt
added a commit
that referenced
this pull request
May 27, 2024
* Fallback to poller replication lag if heartbeat lag fails Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Try to make CI pipeline happy Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Address PR comments Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * Fix typo Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
4 tasks
timvaillancourt
added a commit
that referenced
this pull request
May 28, 2024
* Fallback to poller replication lag if heartbeat lag fails * Try to make CI pipeline happy * Address PR comments * Fix typo --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR should allow to fallback to poller replication lag tracker in case using the heartbeat lag tracker fails. It is meant as a temporary change to allow us to move to heartbeat lag. The reason this is needed is because moving a shard from poller to heartbeat lag tracking can lead to replicas taken out of service. For example:
With the changes in this PR, if there are issues getting the lag from the heartbeat, replication tracker uses poller lag instead, which is what we are currently using.
Once we are fully migrated to using heartbeats, these changes can be reverted.