Check for duplicate nodeids when loading nodes.conf #2852
Conversation
For corrupted (human-made) or program-error-recovered nodes.conf files, check for duplicate nodeids when loading nodes.conf. If a duplicate is found, panic is triggered to prevent nodes from starting up unexpectedly. The node ID is used to identify every node across the whole cluster, we do not expect to find duplicate nodeids in nodes.conf. Signed-off-by: Binbin <[email protected]>
zuiderkwast
left a comment
zuiderkwast
left a comment
There was a problem hiding this comment.
This is very unusual? Only human edited nodes.conf and similar errors?
Signed-off-by: Binbin <[email protected]>
yes, i did manually edited and test it locally, and the node attributes were overridden in various ways. Adding a safeguard doesn't seem like a bad thing. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #2852 +/- ##
============================================
+ Coverage 72.26% 72.35% +0.09%
============================================
Files 128 128
Lines 70370 70382 +12
============================================
+ Hits 50851 50926 +75
+ Misses 19519 19456 -63
🚀 New features to boost your workflow:
|
|
|
||
| zfree(line); | ||
| fclose(fp); | ||
| if (tmp_cluster_nodes) dictRelease(tmp_cluster_nodes); |
There was a problem hiding this comment.
tmp_cluster_nodes is always non-null at this point, right?
There was a problem hiding this comment.
yes, you are right. I will remove it later.
|
|
||
| zfree(line); | ||
| fclose(fp); | ||
| if (tmp_cluster_nodes) dictRelease(tmp_cluster_nodes); |
There was a problem hiding this comment.
Can we add a serverAssert (tmp_cluster_nodes!= NULL) before this
| /* Check if the node (nodeid) has already been loaded. The nodeid is used to | ||
| * identify every node across the entire cluster, we do not expect to find | ||
| * duplicate nodeids in nodes.conf. */ | ||
| sds nodename = sdsnewlen(argv[0], sdslen(argv[0])); |
There was a problem hiding this comment.
do we need to create a new sds for dictfind?
6 participants
For corrupted (human-made) or program-error-recovered nodes.conf files,
check for duplicate nodeids when loading nodes.conf. If a duplicate is
found, panic is triggered to prevent nodes from starting up unexpectedly.
The node ID is used to identify every node across the whole cluster,
we do not expect to find duplicate nodeids in nodes.conf.