Skip to content

More cluster refactoring: move common logic from cluster_legacy.c#3701

Open
zuiderkwast wants to merge 2 commits into
valkey-io:cluster-v2from
zuiderkwast:cluster-v2-refactoring
Open

More cluster refactoring: move common logic from cluster_legacy.c#3701
zuiderkwast wants to merge 2 commits into
valkey-io:cluster-v2from
zuiderkwast:cluster-v2-refactoring

Conversation

@zuiderkwast
Copy link
Copy Markdown
Contributor

@zuiderkwast zuiderkwast commented May 13, 2026

This refactoring is done as a preparation for, and in parallel to, implementing another cluster protocol, so the line between common logic and gossip-specific code is becomming clearer.

Refactor replica migration code in cluster.c: New function clusterHandleLostLastSlot(target) — called when a node loses its last slot in slot migration. Triggers replica migration and does some logging. This code is now called from the protocol-specific code (cluster_legacy.c) when the CLUSTER SETSLOT NODE command has completed removing the last slot from a node.

Replicated CLUSTER SETSLOT / CLUSTER FAILOVER handling (cluster.c). These commands are sometimes received from the primary over the replication stream. Added c == server.primary checks before blocking the client.

Shutdown refactoring (cluster.c, cluster_bus.h, server.c): clusterAutoFailoverOnShutdown moved from cluster_legacy.c to cluster.c (common code). New clusterBus callback prepareShutdown — called early in shutdown for graceful handoff (intended to trigger Raft leader transfer if the leader is shutting down).

Node reconnection logic extracted to cluster_link.c and cluster_link.h. Reconnect code called by common clusterCron. clusterConnectNodes() handles buffer limit checks, handshake timeout removal, reconnection with budget/throttling, and backoff logic.

Node address parsing/serialization extracted (cluster_nodes.c) - parse and build the ip:port@cport,hostname,aux=val string. (To be reused for cluster V2).

getClusterConnectionsCount() simplified. It's not protocol specific.

reset cluster stats code moved to common code.

…common files

This refactoring is done as a preparation to, and in parallel to, implementing another cluster protocol, so the line between common logic and gossip-specific code is becomming clearer.

Refactor replica migration code in cluster.c: New function clusterHandleLostLastSlot(target) — called when a node loses its last slot in slot migration. Triggers replica migration and does some logging. This code is now called from the protocol-specific code (cluster_legacy.c) when the CLUSTER SETSLOT NODE command has completed removing the last slot from a node.

Replicated CLUSTER SETSLOT / CLUSTER FAILOVER handling (cluster.c). These commands are sometimes received from the primary over the replication stream. Added c == server.primary checks before blocking the client.

Shutdown refactoring (cluster.c, cluster_bus.h, server.c): clusterAutoFailoverOnShutdown moved from cluster_legacy.c to cluster.c (common code). New clusterBus callback prepareShutdown — called early in shutdown for graceful handoff (intended to trigger Raft leader transfer if the leader is shutting down).

Node reconnection logic extracted to cluster_link.c and cluster_link.h. Reconnect code called by common clusterCron. clusterConnectNodes() handles buffer limit checks, handshake timeout removal, reconnection with budget/throttling, and backoff logic.

Node address parsing/serialization extracted (cluster_nodes.c) - parse and build the ip:port@cport,hostname,aux=val string. (To be reused for cluster V2).

getClusterConnectionsCount() simplified. It's not protocol specific.

reset cluster stats code moved to common code.

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0dd33b3a-a07c-403c-95aa-f069d7cb19d5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.76%. Comparing base (6f92934) to head (0ff8777).
⚠️ Report is 2 commits behind head on cluster-v2.

Files with missing lines Patch % Lines
src/cluster_nodes.c 80.00% 15 Missing ⚠️
src/cluster.c 99.06% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           cluster-v2    #3701      +/-   ##
==============================================
+ Coverage       76.65%   76.76%   +0.10%     
==============================================
  Files             165      165              
  Lines           80892    80930      +38     
==============================================
+ Hits            62005    62122     +117     
+ Misses          18887    18808      -79     
Files with missing lines Coverage Δ
src/cluster_legacy.c 91.79% <100.00%> (-0.19%) ⬇️
src/cluster_link.c 89.11% <100.00%> (+6.58%) ⬆️
src/server.c 89.47% <100.00%> (-0.03%) ⬇️
src/cluster.c 91.31% <99.06%> (+0.68%) ⬆️
src/cluster_nodes.c 76.02% <80.00%> (+1.94%) ⬆️

... and 23 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.

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast force-pushed the cluster-v2-refactoring branch from c0210a9 to 0ff8777 Compare May 14, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant