valkey
valkey copied to clipboard
Follow up on the capability flag during cluster meet
Thanks for your patience, @bentotten! I think this change makes sense overall. I think we should also consider updating the extension_capable flag right after completing the handshake as below:
https://github.com/valkey-io/valkey/blob/089048d36485bf0d0d202221a7497aec0563bcdd/src/cluster_legacy.c#L3194
otherwise, I think we will have to wait for the next PONG
message to fix the flag at:
https://github.com/valkey-io/valkey/blob/089048d36485bf0d0d202221a7497aec0563bcdd/src/cluster_legacy.c#L3070
it looks like adding the name here during the MEET causes
sender
to be valid during the next packet received, which hits this logic and frees the node/link before the handshake flag can be removed
Yeah, you are right. Setting the name during MEET
alters the handshake sequence. I agree we should not make this change.
Along the same line, I'm now wondering whether we should connect the link
and node
during the MEET
phase (via setClusterNodeToInboundClusterLink
). Currently, we establish these "links" (between link
and node
) after the handshake is completed. Based on the existing implementation, I don't anticipate any issues with moving this logic earlier into the MEET
phase. However, I would be a lot more comfortable if we could find a way to instruct clusterSendPing
to explicitly set this extension-capable flag without relying on link->node
.
Originally posted by @PingXie in https://github.com/valkey-io/valkey/issues/778#issuecomment-2323582206