[
https://issues.apache.org/jira/browse/CASSANDRA-18546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17725686#comment-17725686
]
Stefan Miklosovic edited comment on CASSANDRA-18546 at 5/24/23 8:06 AM:
------------------------------------------------------------------------
To add more to it, I was trying to make sense of the randomization / shuffling
of GossipDigests (by shuffling the endpoints before iterating over them and
adding them to the gDigests list) but I just do not see it. The shuffling like
this tends to try to introduce some non-deterministic behavior when it comes to
the nodes it contacts so the very same nodes are not hit in the same order (if
some of them are significantly slow(er) than the others) but that is not
applicable here either.
In fact, if one looks here (1) and here (2) which calls it, we are picking up a
random node to send GossipDigestSyn message to, which does make sense in the
context of what I wrote in the previous paragraph. However, the fact that we
are even shuffling gossip digests themselves is beyond my comprehension.
Thoughts? [~brandon.williams]
(1)
[https://github.com/apache/cassandra/blob/cassandra-4.0/src/java/org/apache/cassandra/gms/Gossiper.java#L885]
(2)
[https://github.com/apache/cassandra/blob/cassandra-4.0/src/java/org/apache/cassandra/gms/Gossiper.java#L305]
was (Author: smiklosovic):
To add more to it, I was trying to make sense of the randomization / shuffling
of GossipDigests (by shuffling the endpoints before iterating over them and
adding them to the gDigests list) but I just do not see it. The shuffling like
this tends to try to introduce some non-deterministic behavior when it comes to
the nodes it contacts so the very same nodes are not hit in the same order (if
some of them are significantly slow(er) than the others) but that is not
applicable here either.
In fact, if one looks here (1), we are picking up a random node to send
GossipDigestSyn message to, which does make sense in the context of what I
wrote in the previous paragraph. However, the fact that we are even shuffling
gossip digests themselves is beyond my comprehension.
Thoughts? [~brandon.williams]
(1)
https://github.com/apache/cassandra/blob/cassandra-4.0/src/java/org/apache/cassandra/gms/Gossiper.java#L885
> Remove Gossiper#makeRandomGossipDigest
> --------------------------------------
>
> Key: CASSANDRA-18546
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18546
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Cameron Zemek
> Priority: Normal
>
> In going through trying to understand the Gossiper code I come across:
> {code:java}
> /**
> * The gossip digest is built based on randomization
> * rather than just looping through the collection of live endpoints.
> *
> * @param gDigests list of Gossip Digests.
> */
> private void makeRandomGossipDigest(List<GossipDigest> gDigests) {code}
> But I couldn't see what purpose randomization had. In fact in 3.11 it will
> call:
> {code:java}
> doSort(gDigestList); {code}
> On the receiving end, negating the purpose of the randomization.
>
> In discussion with [~stefan.miklosovic] he found this ticket CASSANDRA-14174
> So it seems to me this randomization may have been to allow for limited sizes
> of SYN messages. But this feature doesn't exist and as such by randomizing it
> is:
> * creating more garbage
> * using more CPU (sure its mostly trival; see next point)
> * more time spent on unnecessary functionality on the *single threaded*
> gossip stage.
> * complicating the code and making it more difficult to understand
> In fact there is a bug in the implementation:
> {code:java}
> int generation = 0;
> int maxVersion = 0; // local epstate will be part of
> endpointStateMap
> List<InetAddress> endpoints = new
> ArrayList<InetAddress>(endpointStateMap.keySet());
> Collections.shuffle(endpoints, random);
> for (InetAddress endpoint : endpoints)
> {
> epState = endpointStateMap.get(endpoint);
> if (epState != null)
> {
> generation = epState.getHeartBeatState().getGeneration();
> maxVersion = getMaxEndpointStateVersion(epState);
> }
> gDigests.add(new GossipDigest(endpoint, generation, maxVersion));
> } {code}
> If epState is null and we already had a non-null epState, then the next
> digest will use the generation and maxVersion of the previous iterated
> epState.
>
> Here is change to remove this randomization and fix the above bug,
> [https://github.com/apache/cassandra/pull/2357/commits/1ba422ab5de35f7057c7621ec3607dcbca19768c]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
