Skip to content

Send duplicate multi meet packet only for node which supports it #2840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zuiderkwast merged 1 commit into from
Dec 3, 2025
+28 −15
Merged

Send duplicate multi meet packet only for node which supports it #2840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/cluster_legacy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,8 @@ void clusterUpdateMyselfFlags(void) {
myself->flags |= nofailover;
myself->flags |= CLUSTER_NODE_EXTENSIONS_SUPPORTED |
CLUSTER_NODE_LIGHT_HDR_PUBLISH_SUPPORTED |
CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED;
CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED |
CLUSTER_NODE_MULTI_MEET_SUPPORTED;
if (myself->flags != oldflags) {
clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG | CLUSTER_TODO_UPDATE_STATE);
}
Expand Down Expand Up @@ -3673,6 +3674,13 @@ int clusterProcessPacket(clusterLink *link) {
} else {
sender->flags &= ~CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED;
}

/* Check if the node can handle multi meet packet. */
if (flags & CLUSTER_NODE_MULTI_MEET_SUPPORTED) {
sender->flags |= CLUSTER_NODE_MULTI_MEET_SUPPORTED;
} else {
sender->flags &= ~CLUSTER_NODE_MULTI_MEET_SUPPORTED;
}
Comment on lines +3678 to +3683
Copy link
Contributor

@zuiderkwast zuiderkwast Dec 2, 2025

Choose a reason for hiding this comment

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

We don't really need this code, because CLUSTER_NODE_MULTI_MEET_SUPPORT == CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED and we already have the same code for CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED. We rely on them being equal for the compatibility logic (e.g. when running a mixed cluster with 8.1.0 and 8.1.5).

Copy link
Collaborator Author

@hpatro hpatro Dec 2, 2025

Choose a reason for hiding this comment

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

This is dead code but added for folks not to get confused.

Copy link
Member

@madolson madolson Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can we not have dead code? I was also confuse while reviewing this code. It's better to just have a note that adds the extra explanation no?

Or better yet. Add a high level FEATURE_FLAGS mask and then do;

sender->flage &= ~FEATURE_FLAGS;
sender->flags |= flags & FEATURE_FLAGS;

}

/* Update the last time we saw any data from this node. We
Expand Down Expand Up @@ -5725,7 +5733,8 @@ static int clusterNodeCronHandleReconnect(clusterNode *node, mstime_t now, long
}
if (nodeInNormalState(node) && node->link != NULL && node->inbound_link == NULL &&
now - node->inbound_link_freed_time > getHandshakeTimeout() &&
now - node->meet_sent > getHandshakeTimeout()) {
now - node->meet_sent > getHandshakeTimeout() &&
nodeSupportsMultiMeet(node)) {
/* Node has an outbound link, but no inbound link for more than the handshake timeout.
* This probably means this node does not know us yet, whereas we know it.
* So we send it a MEET packet to do a handshake with it and correct the inconsistent cluster view.
Expand Down
30 changes: 17 additions & 13 deletions src/cluster_legacy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,22 @@ typedef struct clusterLink {
#define linkSupportsExtension(link) ((link)->flags & CLUSTER_LINK_EXTENSIONS_SUPPORTED)

/* Cluster node flags and macros. */
#define CLUSTER_NODE_PRIMARY (1 << 0) /* The node is a primary */
#define CLUSTER_NODE_REPLICA (1 << 1) /* The node is a replica */
#define CLUSTER_NODE_PFAIL (1 << 2) /* Failure? Need acknowledge */
#define CLUSTER_NODE_FAIL (1 << 3) /* The node is believed to be malfunctioning */
#define CLUSTER_NODE_MYSELF (1 << 4) /* This node is myself */
#define CLUSTER_NODE_HANDSHAKE (1 << 5) /* We have still to exchange the first ping */
#define CLUSTER_NODE_NOADDR (1 << 6) /* We don't know the address of this node */
#define CLUSTER_NODE_MEET (1 << 7) /* Send a MEET message to this node */
#define CLUSTER_NODE_MIGRATE_TO (1 << 8) /* Primary eligible for replica migration. */
#define CLUSTER_NODE_NOFAILOVER (1 << 9) /* Replica will not try to failover. */
#define CLUSTER_NODE_EXTENSIONS_SUPPORTED (1 << 10) /* This node supports extensions. */
#define CLUSTER_NODE_LIGHT_HDR_PUBLISH_SUPPORTED (1 << 11) /* This node supports light message header for publish type. */
#define CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED (1 << 12) /* This node supports light message header for module type. */
#define CLUSTER_NODE_PRIMARY (1 << 0) /* The node is a primary */
#define CLUSTER_NODE_REPLICA (1 << 1) /* The node is a replica */
#define CLUSTER_NODE_PFAIL (1 << 2) /* Failure? Need acknowledge */
#define CLUSTER_NODE_FAIL (1 << 3) /* The node is believed to be malfunctioning */
#define CLUSTER_NODE_MYSELF (1 << 4) /* This node is myself */
#define CLUSTER_NODE_HANDSHAKE (1 << 5) /* We have still to exchange the first ping */
#define CLUSTER_NODE_NOADDR (1 << 6) /* We don't know the address of this node */
#define CLUSTER_NODE_MEET (1 << 7) /* Send a MEET message to this node */
#define CLUSTER_NODE_MIGRATE_TO (1 << 8) /* Primary eligible for replica migration. */
#define CLUSTER_NODE_NOFAILOVER (1 << 9) /* Replica will not try to failover. */
#define CLUSTER_NODE_EXTENSIONS_SUPPORTED (1 << 10) /* This node supports extensions. */
#define CLUSTER_NODE_LIGHT_HDR_PUBLISH_SUPPORTED (1 << 11) /* This node supports light message header for publish type. */
#define CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED (1 << 12) /* This node supports light message header for module type. */
#define CLUSTER_NODE_MULTI_MEET_SUPPORTED CLUSTER_NODE_LIGHT_HDR_MODULE_SUPPORTED /* This node handles multi meet packet. \
Light hdr for module and multi meet were both introduced in 8.1, \
so we could reduce the same flag value. */
#define CLUSTER_NODE_NULL_NAME \
"\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" \
"\000\000\000\000\000\000\000\000\000\000\000\000"
Expand All @@ -75,6 +78,7 @@ typedef struct clusterLink {
#define nodeFailed(n) ((n)->flags & CLUSTER_NODE_FAIL)
#define nodeCantFailover(n) ((n)->flags & CLUSTER_NODE_NOFAILOVER)
#define nodeSupportsExtensions(n) ((n)->flags & CLUSTER_NODE_EXTENSIONS_SUPPORTED)
#define nodeSupportsMultiMeet(n) ((n)->flags & CLUSTER_NODE_MULTI_MEET_SUPPORTED)
#define nodeInNormalState(n) (!((n)->flags & (CLUSTER_NODE_HANDSHAKE | CLUSTER_NODE_MEET | CLUSTER_NODE_PFAIL | CLUSTER_NODE_FAIL)))

/* Cluster messages header */
Expand Down
Loading