Skip to content
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.

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

Fix peers v2 system table behaviour when 2 nodes swap their IP Addresses #3263

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

ifesdjeen
Copy link
Contributor

Throw if node id has been changed and does not match directory. If, however the ip address has changed, issue Startup and correct the IP address. Disallow picking over identity of other nodes via hijacking their IPs or via overriding local node id with theirs.

Copy link
Contributor

@beobal beobal left a comment

Choose a reason for hiding this comment

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

just nits

logger.debug("Purging {} from system.peers_v2 table", addresses);
QueryProcessor.executeInternal(String.format(peers_delete_query, SYSTEM_KEYSPACE_NAME, PEERS_V2), addresses.broadcastAddress.getAddress(), addresses.broadcastAddress.getPort());
QueryProcessor.executeInternal(String.format(legacy_peers_delete_query, SYSTEM_KEYSPACE_NAME, LEGACY_PEERS), addresses.broadcastAddress.getAddress());
removeFromLegacyPeerTable(addresses.broadcastAddress);
Copy link
Contributor

Choose a reason for hiding this comment

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

can also replace the calls on lines 171-173 below

@@ -197,4 +195,11 @@ else if (NodeState.isPreJoin(next.directory.peerState(nodeId)))
tokens);
}
}

public static void removeFromLegacyPeerTable(InetAddressAndPort addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: IMO this name is slightly confusing because we execute not only legacy_peers_delete_query but also peers_delete_query. I know that both those tables are "legacy" in the sense that ClusterMetadata.directory is the primary datasource now, but I'd either rename the method to removeFromSystemPeersTables or rename the queries to peers_delete_query / peers_v2_delete_query

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Renamed to removeFromLegacyPeerTable.

Throw if node id has been changed and does not match directory. If, however the _ip_ address has changed, issue Startup and correct the IP address. Disallow picking over identity of other nodes via hijacking their IPs or via overriding local node id with theirs.

Patch by Alex Petrov; reviewed by TBD for CASSANDRA-19221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants