[CASSANDRA-18546] Remove unnecessary shuffling of GossipDigests in Gossiper#makeRandomGossipDigest

• Type: Improvement
• Status: Resolved
• Priority: Normal
• Resolution: Fixed
• Fix Version/s: 4.0.11, 4.1.3, 5.0-alpha1, 5.0
• Component/s: Cluster/Gossip
• Change Category: Semantic
• Complexity: Normal
• Platform: All

Source Control Link

https://github.com/apache/cassandra/commit/f66adb0278aa56e8c2696db6f3fe24d311bf7b44

Test and Documentation Plan

CI

• Assignee: Stefan Miklosovic
• Reporter: Cameron Zemek

Authors

Stefan Miklosovic

Reviewers

Brandon Williams

• Created: 24/May/23 00:50
• Updated: 12/Sep/23 13:02
• Resolved: 30/May/23 21:05

Description

In going through trying to understand the Gossiper code I come across:

    /**
     * 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) 

But I couldn't see what purpose randomization had. In fact in 3.11 it will call:

 doSort(gDigestList); 

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:

        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));
        } 

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

Issue Links

• links to: GitHub Pull Request #2357