diff --git a/src/cluster.c b/src/cluster.c index 9624852..cd60a40 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1406,8 +1406,33 @@ static void SetClusterDataLongForm(RedisModuleString** argv, int argc){ mr_dictEmpty(clusterCtx.nodesMsgIds, NULL); } +static bool IsSameLongFormClusterSet(RedisModuleString** argv, int argc){ + Cluster* current = clusterCtx.CurrCluster; + if (!current || + !current->clusterSetCommand || + current->clusterSetCommandSize != argc) + return false; + + for (int i = 1; i < argc; ++i) { + /* MYID identifies the receiving shard and is not retained in clusterSetCommand. */ + if (i == CLUSTERSET_MYID_LONG_FORM_INDEX) + continue; + size_t argLen; + const char* arg = RedisModule_StringPtrLen(argv[i], &argLen); + if (argLen != strlen(current->clusterSetCommand[i]) || + memcmp(arg, current->clusterSetCommand[i], argLen) != 0) + return false; + } + return true; +} + static int MR_SetClusterData(RedisModuleString** argv, int argc){ if (IsLongFormClusterSet(argc)) { + if (IsSameLongFormClusterSet(argv, argc)) { + RedisModule_Log(mr_staticCtx, "notice", + "Skipping identical long-form cluster set"); + return REDISMODULE_OK; + } SetClusterDataLongForm(argv, argc); return REDISMODULE_OK; } else if (IsShortFormClusterSet(argc)) { diff --git a/tests/mr_test_module/pytests/test_network.py b/tests/mr_test_module/pytests/test_network.py index c62e661..a8d6b3f 100644 --- a/tests/mr_test_module/pytests/test_network.py +++ b/tests/mr_test_module/pytests/test_network.py @@ -211,7 +211,7 @@ def _handle_conn(self, sock, client_addr): conn = Connection(sock) self.new_conns.put(conn) - def _send_cluster_set(self): + def _send_cluster_set(self, mock_shard_id='2', first_arg='NO-USED'): # try to promote to internal connection promote_internal_client_if_supported(env=self.env) # IPv6 endpoints must be bracketed in host:port strings @@ -220,7 +220,7 @@ def _send_cluster_set(self): # argv[6] => myId, argv[7] => "RANGES", argv[8] => numOfRanges, then repeating: # "SHARD" "SLOTRANGE" "ADDR" ["MASTER"] args = [ - 'NO-USED', # [1] + first_arg, # [1] 'NO-USED', # [2] 'NO-USED', # [3] 'NO-USED', # [4] @@ -234,7 +234,7 @@ def _send_cluster_set(self): 'ADDR', 'password@%s:6379' % endpoint_host, 'MASTER', # Shard 2 (mock shard) - 'SHARD', '2', + 'SHARD', mock_shard_id, 'SLOTRANGE', '8193', '16383', 'ADDR', 'password@%s:%d' % (endpoint_host, self.port), 'MASTER' @@ -793,7 +793,30 @@ def testMassiveClusterSet(env, conn): with ShardMock(env, host) as shardMock: for i in range(1000): conn = shardMock.GetConnection(sendHelloResponse=False) - shardMock._send_cluster_set() + # Keep exercising rebuilds now that identical updates are skipped. + shardMock._send_cluster_set(mock_shard_id=str(3 - (i % 2))) + + +@MRTestDecorator(skipOnCluster=True) +def testIdenticalLongFormClusterSetIsNoOp(env, conn): + for host in _get_hosts(): + with ShardMock(env, host) as shardMock: + shardMock.GetConnection() + run_id = env.cmd('MRTESTS.INFOCLUSTER')[3] + + shardMock._send_cluster_set() + env.assertEqual(env.cmd('MRTESTS.INFOCLUSTER')[3], run_id) + + # RedisModuleString can carry embedded NUL bytes. Its explicit length + # must participate in the comparison so this safely rebuilds. + shardMock._send_cluster_set(first_arg=b'NO-USED\0changed') + env.assertNotEqual(env.cmd('MRTESTS.INFOCLUSTER')[3], run_id) + shardMock.GetConnection() + + run_id = env.cmd('MRTESTS.INFOCLUSTER')[3] + shardMock._send_cluster_set(mock_shard_id='3') + env.assertNotEqual(env.cmd('MRTESTS.INFOCLUSTER')[3], run_id) + shardMock.GetConnection() @MRTestDecorator(skipOnCluster=True) def testMassiveClusterSetFromShard(env, conn):