valkey icon indicating copy to clipboard operation
valkey copied to clipboard

Follow up on the capability flag during cluster meet

Open madolson opened this issue 5 months ago • 0 comments

          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

madolson avatar Sep 10 '24 03:09 madolson