Skip to content

Fix meet failure when connecting to a replica - #1435

Merged
Vasileios Zois (vazois) merged 3 commits into
mainfrom
users/matrembl/null-after-failover
Nov 14, 2025
Merged

Vasileios Zois (vazois) merged 3 commits into
mainfrom
users/matrembl/null-after-failover

Conversation

@Mathos1432

@Mathos1432 Mathieu Tremblay (Mathos1432) commented Nov 13, 2025 •

Copy link
Copy Markdown
Contributor

We are seeing failures when trying to run a MEET command on a new node targeting a replica of the cluster.

You effectively issuing a meet on a Replica from a fresh node and that fresh node receives a gossip merge message back to it. Since it is fresh it does not have any information about slots so there is no owner to access, hence ownerId is 0 and this is a special case where NodeId for worker[0] is null

This change adds a check to make sure it no longer fails, and a test to validate the scenario.

@Mathos1432 Mathieu Tremblay (Mathos1432) changed the title Users/matrembl/null after failover Fix meet failure when connecting to a replica Nov 13, 2025
@Mathos1432
Mathieu Tremblay (Mathos1432) marked this pull request as ready for review November 13, 2025 22:51
Copilot AI review requested due to automatic review settings November 13, 2025 22:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a failure that occurs when running a MEET command on a new node targeting a replica of the cluster. The issue was that when a fresh node receives a gossip merge message, it has no slot information, causing ownerId to be 0 (RESERVED_WORKER_ID), which has a null NodeId.

  • Added a null-safety check to prevent accessing workers[0].Nodeid when ownerId is RESERVED_WORKER_ID
  • Added a test case to validate the cluster meet from replica scenario

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libs/cluster/Server/ClusterConfig.cs Added condition to check currentOwnerId != RESERVED_WORKER_ID before accessing workers[currentOwnerId].Nodeid to prevent null reference
test/Garnet.test.cluster/ClusterNegativeTests.cs Added test case ClusterMeetFromReplica to validate the fix by simulating a meet command from a fresh node to a replica

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread libs/cluster/Server/ClusterConfig.cs Outdated
@vazois
Vasileios Zois (vazois) merged commit 54286ac into main Nov 14, 2025
48 of 49 checks passed
@vazois
Vasileios Zois (vazois) deleted the users/matrembl/null-after-failover branch November 14, 2025 05:10
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants