Converge divergent shard-id persisted in nodes.conf to primary's shard id by hpatro · Pull Request #2174 · valkey-io/valkey

hpatro · 2025-06-04T20:25:04Z

Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id.

codecov · 2025-06-04T20:40:15Z

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.57%. Comparing base (3789b29) to head (8f658fe).
Report is 14 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/cluster_legacy.c	0.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2174      +/-   ##
============================================
+ Coverage     71.43%   73.57%   +2.13%     
============================================
  Files           122      122              
  Lines         66210    73777    +7567     
============================================
+ Hits          47300    54281    +6981     
- Misses        18910    19496     +586

Files with missing lines	Coverage Δ
src/cluster_legacy.c	`89.50% <0.00%> (+2.77%)`	⬆️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/cluster_legacy.c

Signed-off-by: Harkrishn Patro <[email protected]>

hpatro · 2025-06-11T17:39:49Z

@martinrvisser I tried few scenarios manually, they seem to run fine. Could you try running this patch and see if it helps ?

hpatro · 2025-06-11T17:43:03Z

Test scenarios:

43ee1cacd6948ee96bb367eb8795e62e8d153f05 10.125.153.181:0@28007,,tls-port=18007,shard-id=f91532eb722943e035f34292cf586f3f750d65bd myself,master - 0 1749488968682 13 connected 5461-16383
8d89f4d4e7c57a2819277732f86213241c3ec0d3 10.124.45.182:0@28007,,tls-port=18007,shard-id=a91532eb722943e035f34292cf586f3f750d65bd slave 43ee1cacd6948ee96bb367eb8795e62e8d153f05 0 1749488968682 13 connected
vars currentEpoch 13 lastVoteEpoch 9

8d89f4d4e7c57a2819277732f86213241c3ec0d3 10.124.45.182:0@28007,,tls-port=18007,shard-id=a91532eb722943e035f34292cf586f3f750d65bd slave 43ee1cacd6948ee96bb367eb8795e62e8d153f05 0 1749488968682 13 connected
43ee1cacd6948ee96bb367eb8795e62e8d153f05 10.125.153.181:0@28007,,tls-port=18007,shard-id=f91532eb722943e035f34292cf586f3f750d65bd myself,master - 0 1749488968682 13 connected 5461-16383
vars currentEpoch 13 lastVoteEpoch 9

43ee1cacd6948ee96bb367eb8795e62e8d153f05 10.125.153.181:0@28007,,tls-port=18007,shard-id=f91532eb722943e035f34292cf586f3f750d65bd master - 0 1749488968682 13 connected 5461-16383
8d89f4d4e7c57a2819277732f86213241c3ec0d3 10.124.45.182:0@28007,,tls-port=18007,shard-id=a91532eb722943e035f34292cf586f3f750d65bd myself,slave 43ee1cacd6948ee96bb367eb8795e62e8d153f05 0 1749488968682 13 connected
vars currentEpoch 13 lastVoteEpoch 9

8d89f4d4e7c57a2819277732f86213241c3ec0d3 10.124.45.182:0@28007,,tls-port=18007,shard-id=a91532eb722943e035f34292cf586f3f750d65bd myself,slave 43ee1cacd6948ee96bb367eb8795e62e8d153f05 0 1749488968682 13 connected
43ee1cacd6948ee96bb367eb8795e62e8d153f05 10.125.153.181:0@28007,,tls-port=18007,shard-id=f91532eb722943e035f34292cf586f3f750d65bd master - 0 1749488968682 13 connected 5461-16383
vars currentEpoch 13 lastVoteEpoch 9

All of these load fine.

martinrvisser · 2025-06-13T16:15:07Z

@martinrvisser I tried few scenarios manually, they seem to run fine. Could you try running this patch and see if it helps ?

@hpatro fine in my tests also, nice work
2804808:M 13 Jun 2025 12:13:25.599 * Node 386bac3e5337540f2bce4181ff8980f33a989667 has a different shard id (4009ad6c27a3fe91b709a6502bd111b5d58eada0) than its primary's shard id bb85c3d913bb6f9e3d02e68ae89b5c5e61111fb4 (f713a7d0f3079e30fcc47348260a09de0c9b8b72). Updating replica's shard id to match primary's shard id.
2804808:M 13 Jun 2025 12:13:25.599 * Node configuration loaded, I'm 386bac3e5337540f2bce4181ff8980f33a989667

Signed-off-by: Harkrishn Patro <[email protected]>

enjoy-binbin

This look correct, please also update the top comment to desc it. So it is somehow like a corrupted (or a outupdated) nodes.conf cause the trouble?

hpatro · 2025-06-18T23:52:09Z

This look correct, please also update the top comment to desc it. So it is somehow like a corrupted (or a outupdated) nodes.conf cause the trouble?

Yes. Updated.

enjoy-binbin

LGTM

Signed-off-by: Kyle J. Davis <[email protected]>

…d id (valkey-io#2174) Fixes valkey-io#2171 Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id. --------- Signed-off-by: Harkrishn Patro <[email protected]> Signed-off-by: Viktor Söderqvist <[email protected]>

…d id (#2174) Fixes #2171 Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id. --------- Signed-off-by: Harkrishn Patro <[email protected]> Signed-off-by: Viktor Söderqvist <[email protected]>

…d id (valkey-io#2174) Fixes valkey-io#2171 Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id. --------- Signed-off-by: Harkrishn Patro <[email protected]> Signed-off-by: Viktor Söderqvist <[email protected]>

…d id (valkey-io#2174) Fixes valkey-io#2171 Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id. --------- Signed-off-by: Harkrishn Patro <[email protected]>

zuiderkwast · 2025-09-30T12:53:33Z

I tried backporting this one to 7.2 but I couldn't get the test case to pass, so I'm skipping it for now. If anyone wants to help backporting it, feel free to open a PR targeting the 7.2 branch. Some difficulties to note:

src/cluster_legacy.c doesn't exist in 7.2. It's still called src/cluster.c here.
Variables for primaries and replicas are using the old names masters and slaves.

…d id (#2174) Fixes #2171 Handle divergent shard-id across primary and replica from nodes.conf and reconcile all the nodes in the shard to the primary node's shard-id. --------- Signed-off-by: Harkrishn Patro <[email protected]>

… id (valkey-io#2174) Handle divergent shard-id across master and replica from nodes.conf and reconcile all the nodes in the shard to the master node's shard-id. Fixes valkey-io#2171 (cherry picked from commit 4a3e713) Signed-off-by: Harkrishn Patro <[email protected]>

… id (valkey-io#2174) Handle divergent shard-id across master and replica from nodes.conf and reconcile all the nodes in the shard to the master node's shard-id. Fixes valkey-io#2171 (cherry picked from commit 4a3e713) Signed-off-by: Harkrishn Patro <[email protected]> Signed-off-by: Roshan Khatri <[email protected]>

roshkhatri · 2026-02-18T22:09:54Z

I tried backporting this one to 7.2 but I couldn't get the test case to pass, so I'm skipping it for now. If anyone wants to help backporting it, feel free to open a PR targeting the 7.2 branch. Some difficulties to note:
src/cluster_legacy.c doesn't exist in 7.2. It's still called src/cluster.c here.
Variables for primaries and replicas are using the old names masters and slaves.

@zuiderkwast opened a PR for this here: #3229

#3229) Backporting #2174 to 7.2 branch Handle divergent shard-id across master and replica from nodes.conf and reconcile all the nodes in the shard to the master node's shard-id. Fixes #2171 (cherry picked from commit 4a3e713) Signed-off-by: Harkrishn Patro <[email protected]> Signed-off-by: Roshan Khatri <[email protected]>

hpatro requested a review from PingXie June 4, 2025 20:25

hwware approved these changes Jun 5, 2025

View reviewed changes

hpatro mentioned this pull request Jun 6, 2025

[BUG] "Unrecoverable error: corrupted cluster config file" due to incorrect shard-id #2171

Closed

PingXie reviewed Jun 9, 2025

View reviewed changes

src/cluster_legacy.c Show resolved Hide resolved

hpatro added 2 commits June 10, 2025 20:26

Handle divergent shard-id persisted in nodes.conf for a particular shard

39dfe46

Signed-off-by: Harkrishn Patro <[email protected]>

Add log lines when divergent shard information is reconciled

8f658fe

Signed-off-by: Harkrishn Patro <[email protected]>

hpatro force-pushed the handle-divergent-shard-id-nodes-conf branch from 5c52a9d to 8f658fe Compare June 10, 2025 20:27

Add tests

4cae442

Signed-off-by: Harkrishn Patro <[email protected]>

hpatro requested review from PingXie, enjoy-binbin and madolson June 17, 2025 21:11

Update tests

38cb829

Signed-off-by: Harkrishn Patro <[email protected]>

enjoy-binbin reviewed Jun 18, 2025

View reviewed changes

enjoy-binbin approved these changes Jun 19, 2025

View reviewed changes

hpatro added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Jun 20, 2025

hpatro merged commit 4a3e713 into valkey-io:unstable Jun 20, 2025
81 checks passed

hpatro added this to Valkey 8.1, Valkey 8.0 and Valkey 7.2 Jul 16, 2025

hpatro mentioned this pull request Aug 13, 2025

Valkey 9.0.0-rc1 release notes and version.h bump #2476

Merged

stockholmux added a commit to stockholmux/valkey that referenced this pull request Aug 13, 2025

adds valkey-io#1466 and valkey-io#2174

300e8ee

Signed-off-by: Kyle J. Davis <[email protected]>

hpatro added the release-notes This issue should get a line item in the release notes label Aug 13, 2025

zuiderkwast moved this to 8.0.5 (to be released) in Valkey 8.0 Aug 21, 2025

zuiderkwast moved this to To be backported in Valkey 8.1 Aug 21, 2025

zuiderkwast moved this to To be backported in Valkey 7.2 Aug 21, 2025

zuiderkwast mentioned this pull request Aug 21, 2025

Fixes for Valkey 8.0.5 #2210

Merged

ranshid moved this from To be backported to In Progress in Valkey 8.1 Sep 30, 2025

ranshid moved this from In Progress to To be backported in Valkey 8.1 Sep 30, 2025

zuiderkwast moved this from To be backported to 8.1.4 in Valkey 8.1 Oct 1, 2025

roshkhatri mentioned this pull request Feb 18, 2026

Converge divergent shard-id persisted in nodes.conf to master's shard… #3229

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converge divergent shard-id persisted in nodes.conf to primary's shard id#2174

Converge divergent shard-id persisted in nodes.conf to primary's shard id#2174
hpatro merged 4 commits intovalkey-io:unstablefrom
hpatro:handle-divergent-shard-id-nodes-conf

hpatro commented Jun 4, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jun 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

hpatro commented Jun 11, 2025

Uh oh!

hpatro commented Jun 11, 2025

Uh oh!

martinrvisser commented Jun 13, 2025

Uh oh!

enjoy-binbin left a comment

Uh oh!

hpatro commented Jun 18, 2025

Uh oh!

enjoy-binbin left a comment

Uh oh!

Uh oh!

zuiderkwast commented Sep 30, 2025

Uh oh!

roshkhatri commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

hpatro commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

hpatro commented Jun 11, 2025

Uh oh!

hpatro commented Jun 11, 2025

Uh oh!

martinrvisser commented Jun 13, 2025

Uh oh!

enjoy-binbin left a comment

Choose a reason for hiding this comment

Uh oh!

hpatro commented Jun 18, 2025

Uh oh!

enjoy-binbin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zuiderkwast commented Sep 30, 2025

Uh oh!

roshkhatri commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hpatro commented Jun 4, 2025 •

edited

Loading

codecov bot commented Jun 4, 2025 •

edited

Loading