-
Notifications
You must be signed in to change notification settings - Fork 85
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
Blocking command port when node is not caught up #2039
Blocking command port when node is not caught up #2039
Conversation
sqlitecluster/SQLiteNode.h
Outdated
atomic<SQLiteNodeState> _state; | ||
|
||
// Keeps track if we have closed the command port for commits fallen behind. | ||
bool _blockedCommandPort{false}; |
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.
Since we block the command port for other reasons, we should put the fact that this is being set based on the node being behind on the variable name, for clarity
bool _blockedCommandPort{false}; | |
bool _blockedCommandPortForBeingBehind{false}; |
But do we really need this variable? Is there an easy way to check the reason the command port is blocked so we can do something like
_blockedCommandPortForBeingBehind = getReason() == "COMMITS_LAGGING_BEHIND";
if (peer == _leadPeer && currentCommitDifference >= 12'500 && !_blockedCommandPort) {
...
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.
But do we really need this variable? Is there an easy way to check the reason the command port is blocked so we can do something like
tl;dr: Adding this as a separate variable so we don't need to add complexity to the current methods related to block reasons. We had an implementation like you're suggesting before, but that would require adding new locks/changing current lock types.
This was discussed in this thread: https://expensify.slack.com/archives/CC7NECV4L/p1734981555759429?thread_ts=1733270912.693829&cid=CC7NECV4L
Co-authored-by: Carlos Alvarez <[email protected]>
Co-authored-by: Carlos Alvarez <[email protected]>
Details
This PR blocks the command port if a node is more than 12.5K commits behind the leader, and reopens when we're 1K behind.
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/437855
Tests
Internal Testing Reminder: when changing bedrock, please compile auth against your new changes