Flip pfail flag while marking node as failed #2012
Conversation
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #2012 +/- ##
============================================
- Coverage 71.01% 71.00% -0.01%
============================================
Files 123 123
Lines 66033 66115 +82
============================================
+ Hits 46892 46945 +53
- Misses 19141 19170 +29
🚀 New features to boost your workflow:
|
zuiderkwast
left a comment
zuiderkwast
left a comment
There was a problem hiding this comment.
Why?
Can this difference be observed by users at all?
As far as I understand, the cluster node state transition from PFAIL to FAIL and both of these are mutually exclusive and shouldn't exist together. So, I think finding the below response under > ./valkey-cli cluster nodes | grep fail?
511676acd16696d5b8767aa6c4a8ba54c69b70a0 :0@0 master,fail?,fail,noaddr - 1745372213541 0 1993 disconnected |
|
looks like we do a similar change here in markNodeAsFailingIfNeeded. It is called in clusterCron and through cluster gossip. |
zuiderkwast
left a comment
zuiderkwast
left a comment
There was a problem hiding this comment.
Yeah, makes sense to me.
enjoy-binbin
left a comment
enjoy-binbin
left a comment
There was a problem hiding this comment.
LGTM. btw, how do you reproduce it?
|
Backport this one? Is it a follow-up of #1191 which is included in 8.1. |
This fail logic was added in valkey-io#1191, we should also clear the pfail flag in this case. Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com> Signed-off-by: chzhoo <czawyx@163.com>
This fail logic was added in valkey-io#1191, we should also clear the pfail flag in this case. Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com> Signed-off-by: shanwan1 <shanwan1@intel.com>
5 participants
This fail logic was added in #1191, we should also clear the
pfail flag in this case.