-
Notifications
You must be signed in to change notification settings - Fork 687
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
Minor log fixes when failover auth denied due to slot epoch #1341
base: unstable
Are you sure you want to change the base?
Conversation
The old reqEpoch mainly refers to requestCurrentEpoch, see: ``` if (requestCurrentEpoch < server.cluster->currentEpoch) { serverLog(LL_WARNING, "Failover auth denied to %.40s (%s): reqEpoch (%llu) < curEpoch(%llu)", node->name, node->human_nodename, (unsigned long long)requestCurrentEpoch, (unsigned long long)server.cluster->currentEpoch); return; } ``` And in here we refer to requestConfigEpoch, it's a bit misleading, so change it to reqConfigEpoch to make it clear. Signed-off-by: Binbin <[email protected]>
serverLog(LL_NOTICE, "Starting a failover election for epoch %llu, node epoch is %llu", | ||
(unsigned long long)server.cluster->currentEpoch, (unsigned long long)nodeEpoch(myself)); |
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.
I am struggling with the changes here. If it is not needed, you can use the review suggestion to remove it.
serverLog(LL_NOTICE, "Starting a failover election for epoch %llu, node epoch is %llu", | |
(unsigned long long)server.cluster->currentEpoch, (unsigned long long)nodeEpoch(myself)); | |
serverLog(LL_NOTICE, "Starting a failover election for epoch %llu.", | |
(unsigned long long)server.cluster->currentEpoch); |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1341 +/- ##
============================================
+ Coverage 70.77% 70.78% +0.01%
============================================
Files 116 116
Lines 63280 63280
============================================
+ Hits 44787 44794 +7
+ Misses 18493 18486 -7
|
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.
I have no concerns with this.
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.
Yes, if you need this info in the log, then I think you're right.
The old reqEpoch mainly refers to requestCurrentEpoch, see:
And in here we refer to requestConfigEpoch, it's a bit misleading,
so change it to reqConfigEpoch to make it clear.