Repository: valkey-io/valkey
File: src/cluster_legacy.c (C)

Code before:
        listIter li;
        listNode *ln;
        int legacy_replica_found = 0;
        int online_replica_found = 0;
        listRewind(server.replicas, &li);
        while ((ln = listNext(&li))) {
            client *r = ln->value;
            if (r->replica_version < 0x702ff /* 7.2.255 */) {
                legacy_replica_found++;
            }
            if (r->repl_state == REPLICA_STATE_ONLINE) {
                online_replica_found++;
            }
        }

        if (!legacy_replica_found && online_replica_found) {
            forceCommandPropagation(c, PROPAGATE_REPL);
            /* We are a primary and this is the first time we see this `SETSLOT`
             * command. Force-replicate the command to all of our replicas
             * first and only on success we will handle the command.
             * Note that
             * 1. All replicas are expected to ack the replication within the given timeout
             * 2. The repl offset target is set to the primary's current repl offset + 1.
             *    There is no concern of partial replication because replicas always
             *    ack the repl offset at the command boundary. */
            blockClientForReplicaAck(c, timeout_ms, server.primary_repl_offset + 1, online_replica_found, 0);
            /* Mark client as pending command for execution after replication to replicas. */
            c->flag.pending_command = 1;
            replicationRequestAckFromReplicas();
            return;
        }
    }

    /* Slot states have been updated on the compatible replicas (if any).
     * Now execute the command on the primary. */
    if (!strcasecmp(c->argv[3]->ptr, "migrating")) {
        serverLog(LL_NOTICE, "Migrating slot %d to node %.40s (%s)", slot, n->name, n->human_nodename);
        server.cluster->migrating_slots_to[slot] = n;
    } else if (!strcasecmp(c->argv[3]->ptr, "importing")) {
        serverLog(LL_NOTICE, "Importing slot %d from node %.40s (%s)", slot, n->name, n->human_nodename);
        server.cluster->importing_slots_from[slot] = n;
    } else if (!strcasecmp(c->argv[3]->ptr, "stable")) {
        /* CLUSTER SETSLOT <SLOT> STABLE */
        serverLog(LL_NOTICE, "Marking slot %d stable", slot);
        server.cluster->importing_slots_from[slot] = NULL;
        server.cluster->migrating_slots_to[slot] = NULL;
    } else if (!strcasecmp(c->argv[3]->ptr, "node")) {
        /* CLUSTER SETSLOT <SLOT> NODE <NODE ID> */
        serverLog(LL_NOTICE, "Assigning slot %d to node %.40s (%s) in shard %.40s", slot, n->name, n->human_nodename,
                  n->shard_id);


Review comment (suggestion):

```suggestion
            blockClientForReplicaAck(c, timeout_ms, server.primary_repl_offset + 1, num_eligible_replicas, 0);
```


Code after:
        listIter li;
        listNode *ln;
        int num_eligible_replicas = 0;
        listRewind(server.replicas, &li);
        while ((ln = listNext(&li))) {
            client *r = ln->value;

            /* We think that when the command comes in, the primary only needs to
             * wait for the online replicas. The admin can easily check if there
             * are replicas that are down for an extended period of time. If they
             * decide to move forward anyways, we should not block it. If a replica
             * failed right before the replication and was not included in the
             * replication, it would also unlikely win the election.
             *
             * And 0x702ff is 7.2.255, we only support new versions in this case. */
            if (r->repl_state == REPLICA_STATE_ONLINE && r->replica_version > 0x702ff) {
                num_eligible_replicas++;
            }
        }

        if (num_eligible_replicas != 0) {
            forceCommandPropagation(c, PROPAGATE_REPL);
            /* We are a primary and this is the first time we see this `SETSLOT`
             * command. Force-replicate the command to all of our replicas
             * first and only on success will we handle the command.
             * Note that
             * 1. All replicas are expected to ack the replication within the given timeout
             * 2. The repl offset target is set to the primary's current repl offset + 1.
             *    There is no concern of partial replication because replicas always
             *    ack the repl offset at the command boundary. */
            blockClientForReplicaAck(c, timeout_ms, server.primary_repl_offset + 1, num_eligible_replicas, 0);
            /* Mark client as pending command for execution after replication to replicas. */
            c->flag.pending_command = 1;
            replicationRequestAckFromReplicas();
            return;
        }
    }

    /* Slot states have been updated on the compatible replicas (if any).
     * Now execute the command on the primary. */
    if (!strcasecmp(c->argv[3]->ptr, "migrating")) {
        serverLog(LL_NOTICE, "Migrating slot %d to node %.40s (%s)", slot, n->name, n->human_nodename);
        server.cluster->migrating_slots_to[slot] = n;
    } else if (!strcasecmp(c->argv[3]->ptr, "importing")) {
        serverLog(LL_NOTICE, "Importing slot %d from node %.40s (%s)", slot, n->name, n->human_nodename);
        server.cluster->importing_slots_from[slot] = n;
    } else if (!strcasecmp(c->argv[3]->ptr, "stable")) {
        /* CLUSTER SETSLOT <SLOT> STABLE */
        serverLog(LL_NOTICE, "Marking slot %d stable", slot);
        server.cluster->importing_slots_from[slot] = NULL;
        server.cluster->migrating_slots_to[slot] = NULL;