Use dictGetSomeKeys instead of dictGetRandomKey for gossip node population

src/cluster_legacy.c

@@ -4796,11 +4796,21 @@
             link->node->meet_sent = mstime();
     }
 
-    /* Populate the gossip fields */
-    int maxiterations = wanted * 3;
-    while (freshnodes > 0 && gossipcount < wanted && maxiterations--) {
-        dictEntry *de = dictGetRandomKey(server.cluster->nodes);
-        clusterNode *this = dictGetVal(de);
+    /* Populate the gossip fields.
+     * Use dictGetSomeKeys() to sample candidates in a single batch instead
+     * of calling dictGetRandomKey() in a retry loop. We over-allocate to
+     * have enough candidates after filtering out ineligible nodes.
+     * dictGetSomeKeys() picks a random starting point each call, so over
+     * many ping rounds all nodes get even coverage without needing an
+     * explicit shuffle. */
+    int candidates_wanted = wanted + 2; /* +2 for myself and link->node */
+    if (candidates_wanted > (int)dictSize(server.cluster->nodes))
+        candidates_wanted = dictSize(server.cluster->nodes);
+    dictEntry **candidates = zmalloc(sizeof(dictEntry *) * candidates_wanted);
+    unsigned int ncandidates = dictGetSomeKeys(server.cluster->nodes, candidates, candidates_wanted);
+
+    for (unsigned int i = 0; i < ncandidates && gossipcount < wanted; i++) {
+        clusterNode *this = dictGetVal(candidates[i]);
 
         /* Don't include this node: the whole packet header is about us
          * already, so we just gossip about other nodes.
@@ -4818,7 +4828,6 @@
          */
         if (this->flags & (CLUSTER_NODE_HANDSHAKE | CLUSTER_NODE_NOADDR) ||
             (this->link == NULL && this->numslots == 0)) {
-            freshnodes--; /* Technically not correct, but saves CPU. */
             continue;
         }
 
@@ -4828,9 +4837,9 @@
         /* Add it */
         clusterSetGossipEntry(hdr, gossipcount, this);
         this->last_in_ping_gossip = cluster_pings_sent;
-        freshnodes--;
         gossipcount++;
     }
+    zfree(candidates);
 
     /* If there are PFAIL nodes, add them at the end. */
     if (pfail_wanted) {

src/unit/test_dict.c

@@ -325,3 +325,72 @@ int test_dictBenchmark(int argc, char **argv, int flags) {
     dictRelease(dict);
     return 0;
 }
+
+/* Benchmark comparing the gossip-style random sampling approaches:
+ * 1) Calling dictGetRandomKey() in a loop (old approach)
+ * 2) Calling dictGetSomeKeys() once as a batch (new approach)
+ *
+ * Both simulate selecting `wanted` random entries from a dict of `count`
+ * entries, mirroring the gossip field population in cluster_legacy.c. */
+int test_dictBenchmarkGetRandomKeyVsGetSomeKeys(int argc, char **argv, int flags) {
+    long long start, elapsed;
+    int accurate = (flags & UNIT_TEST_ACCURATE);
+    long count = accurate ? 1000000 : 2000;
+    long iterations = accurate ? 1000 : 50000;
+
+    UNUSED(argc);
+    UNUSED(argv);
+
+    monotonicInit();
+
+    /* Build a dict with `count` entries. */
+    dict *d = dictCreate(&BenchmarkDictType);
+    for (long i = 0; i < count; i++) {
+        int retval = dictAdd(d, stringFromLongLong(i), (void *)i);
+        TEST_ASSERT(retval == DICT_OK);
+    }
+    while (dictIsRehashing(d)) dictRehashMicroseconds(d, 100 * 1000);
+
+    /* `wanted` is 10% of total nodes, matching the default gossip config. */
+    int wanted = (int)(count * 0.10);
+
+    /* --- Approach 1: dictGetRandomKey() in a retry loop --- */
+    start_benchmark();
+    for (long iter = 0; iter < iterations; iter++) {
+        int gossipcount = 0;
+        int maxiterations_inner = wanted * 3;
+        while (gossipcount < wanted && maxiterations_inner--) {
+            dictEntry *de = dictGetRandomKey(d);
+            TEST_ASSERT(de != NULL);
+            /* In real code we'd filter here; just count to simulate the work. */
+            gossipcount++;
+        }
+        TEST_ASSERT(gossipcount == wanted);
+    }
+    end_benchmark("dictGetRandomKey loop");
+    long long elapsed_random_key = elapsed;
+
+    /* --- Approach 2: dictGetSomeKeys() batch --- */
+    int candidates_wanted = wanted + 2;
+    dictEntry **candidates = zmalloc(sizeof(dictEntry *) * candidates_wanted);
+
+    start_benchmark();
+    for (long iter = 0; iter < iterations; iter++) {
+        unsigned int ncandidates = dictGetSomeKeys(d, candidates, candidates_wanted);
+        TEST_ASSERT(ncandidates > 0);
+        int gossipcount = 0;
+        for (unsigned int i = 0; i < ncandidates && gossipcount < wanted; i++) {
+            TEST_ASSERT(candidates[i] != NULL);
+            gossipcount++;
+        }
+    }
+    end_benchmark("dictGetSomeKeys batch");
+    long long elapsed_some_keys = elapsed;
+
+    printf("Speedup: dictGetSomeKeys is %.2fx faster\n",
+           elapsed_some_keys > 0 ? (double)elapsed_random_key / (double)elapsed_some_keys : 0.0);
+
+    zfree(candidates);
+    dictRelease(d);
+    return 0;
+}

src/unit/test_files.h

@@ -19,6 +19,7 @@ int test_dictEmptyDirAdd128Keys(int argc, char **argv, int flags);
 int test_dictDisableResizeReduceTo3(int argc, char **argv, int flags);
 int test_dictDeleteOneKeyTriggerResizeAgain(int argc, char **argv, int flags);
 int test_dictBenchmark(int argc, char **argv, int flags);
+int test_dictBenchmarkGetRandomKeyVsGetSomeKeys(int argc, char **argv, int flags);
 int test_endianconv(int argc, char *argv[], int flags);
 int test_entryCreate(int argc, char **argv, int flags);
 int test_entryUpdate(int argc, char **argv, int flags);
@@ -289,7 +290,7 @@ int test_zmallocAllocZeroByteAndFree(int argc, char **argv, int flags);
 unitTest __test_bitops_c[] = {{"test_popcount", test_popcount}, {NULL, NULL}};
 unitTest __test_crc64_c[] = {{"test_crc64", test_crc64}, {NULL, NULL}};
 unitTest __test_crc64combine_c[] = {{"test_crc64combine", test_crc64combine}, {NULL, NULL}};
-unitTest __test_dict_c[] = {{"test_dictCreate", test_dictCreate}, {"test_dictAdd16Keys", test_dictAdd16Keys}, {"test_dictDisableResize", test_dictDisableResize}, {"test_dictAddOneKeyTriggerResize", test_dictAddOneKeyTriggerResize}, {"test_dictDeleteKeys", test_dictDeleteKeys}, {"test_dictDeleteOneKeyTriggerResize", test_dictDeleteOneKeyTriggerResize}, {"test_dictEmptyDirAdd128Keys", test_dictEmptyDirAdd128Keys}, {"test_dictDisableResizeReduceTo3", test_dictDisableResizeReduceTo3}, {"test_dictDeleteOneKeyTriggerResizeAgain", test_dictDeleteOneKeyTriggerResizeAgain}, {"test_dictBenchmark", test_dictBenchmark}, {NULL, NULL}};
+unitTest __test_dict_c[] = {{"test_dictCreate", test_dictCreate}, {"test_dictAdd16Keys", test_dictAdd16Keys}, {"test_dictDisableResize", test_dictDisableResize}, {"test_dictAddOneKeyTriggerResize", test_dictAddOneKeyTriggerResize}, {"test_dictDeleteKeys", test_dictDeleteKeys}, {"test_dictDeleteOneKeyTriggerResize", test_dictDeleteOneKeyTriggerResize}, {"test_dictEmptyDirAdd128Keys", test_dictEmptyDirAdd128Keys}, {"test_dictDisableResizeReduceTo3", test_dictDisableResizeReduceTo3}, {"test_dictDeleteOneKeyTriggerResizeAgain", test_dictDeleteOneKeyTriggerResizeAgain}, {"test_dictBenchmark", test_dictBenchmark}, {"test_dictBenchmarkGetRandomKeyVsGetSomeKeys", test_dictBenchmarkGetRandomKeyVsGetSomeKeys}, {NULL, NULL}};
 unitTest __test_endianconv_c[] = {{"test_endianconv", test_endianconv}, {NULL, NULL}};
 unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entryUpdate", test_entryMemUsage_entrySetExpiry_entryUpdate}, {"test_entryStringRef", test_entryStringRef}, {NULL, NULL}};
 unitTest __test_fifo_c[] = {{"test_fifoEmptyPop", test_fifoEmptyPop}, {"test_fifoEmptyPeek", test_fifoEmptyPeek}, {"test_fifoSimplePushPop", test_fifoSimplePushPop}, {"test_fifoTryVariousSizes", test_fifoTryVariousSizes}, {"test_fifoPushPopTest", test_fifoPushPopTest}, {"test_fifoJoinTest", test_fifoJoinTest}, {"test_fifoComparePerformance", test_fifoComparePerformance}, {NULL, NULL}};