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

Code before:
/* Set replication to the specified primary address and port. */
void replicationSetPrimary(char *ip, int port, int full_sync_required) {
    int was_primary = server.primary_host == NULL;

    sdsfree(server.primary_host);
    server.primary_host = NULL;
    if (server.primary) {
        /* When joining 'myself' to a new primary, set the dont_cache_primary flag
         * if a full sync is required. This happens when 'myself' was previously
         * part of a different shard from the new primary. Since 'myself' does not
         * have the replication history of the shard it is joining, clearing the
         * cached primary is necessary to ensure proper replication behavior. */
        server.primary->flag.dont_cache_primary = full_sync_required;
        freeClient(server.primary);
    }

    /* Setting primary_host only after the call to freeClient since it calls
     * replicationHandlePrimaryDisconnection which can trigger a re-connect
     * directly from within that call. */
    server.primary_host = sdsnew(ip);
    server.primary_port = port;

    /* Clients blocked in primary, now replica.
     * Note that we need to set the new primary first
     * since unblocking clients require redirecting to the new primary */
    disconnectAllBlockedClients();

    /* Update oom_score_adj */
    setOOMScoreAdj(-1);

    /* Here we don't disconnect with replicas, since they may hopefully be able
     * to partially resync with us. We will disconnect with replicas and force
     * them to resync with us when changing replid on partially resync with new
     * primary, or finishing transferring RDB and preparing loading DB on full
     * sync with new primary. */

    cancelReplicationHandshake(0);

    /* Before destroying our primary state, create a cached primary using
     * our own parameters, to later PSYNC with the new primary. */
    if (was_primary && !full_sync_required) {
        replicationDiscardCachedPrimary();
        replicationCachePrimaryUsingMyself();
    }

    /* Fire the role change modules event. */
    moduleFireServerEvent(VALKEYMODULE_EVENT_REPLICATION_ROLE_CHANGED, VALKEYMODULE_EVENT_REPLROLECHANGED_NOW_REPLICA,
                          NULL);

    /* Fire the primary link modules event. */
    if (server.repl_state == REPL_STATE_CONNECTED)

Review comment (bug):
I proposed the same functionality for REDIRECT a while ago, see https://github.com/valkey-io/valkey/pull/848/. Redirecting the client here is too early in the failover case:

1. The role switch hasn't happened in `FAILOVER_IN_PROGESS` yet,
    thus, there is no new primary to connect to. The designated primary is
   still a replica and wil promptly redirect the client back to this node.
2. The failover may still fail. In this case, there is no need to
    unblock/disconnect clients at all.

Therefore, https://github.com/valkey-io/valkey/pull/848/ moved the `disconnectAllBlockedClients` to after the failover completes successfully and the new primary is ready to accept clients.
 


Code after:
     * and the global offset, to try a partial resync at the next
     * reconnection attempt. */
    case REPL_STATE_SEND_PSYNC:
    case_send_psync:
        if (syncWithPrimaryHandleSendPsyncState(conn) == C_ERR) {
            syncWithPrimaryHandleError(&conn);
            return;
        }
        server.repl_state = REPL_STATE_RECEIVE_PSYNC_REPLY;
        return;
    /* If reached this point, we should be in REPL_STATE_RECEIVE_PSYNC_REPLY. */
    default:
        if (server.repl_state != REPL_STATE_RECEIVE_PSYNC_REPLY) {
            serverLog(LL_WARNING,
                      "syncWithPrimary(): state machine error, "
                      "state should be RECEIVE_PSYNC but is %d",
                      server.repl_state);
            syncWithPrimaryHandleError(&conn);
            return;
        }
    }

    psync_result = replicaProcessPsyncReply(conn);
    if (psync_result == PSYNC_WAIT_REPLY) return; /* Try again later... */

    /* Check the status of the planned failover. We expect PSYNC_CONTINUE,
     * but there is nothing technically wrong with a full resync which
     * could happen in edge cases. */
    if (server.failover_state == FAILOVER_IN_PROGRESS) {
        if (psync_result == PSYNC_CONTINUE || psync_result == PSYNC_FULLRESYNC) {
            clearFailoverState(true);
        } else {
            abortFailover("Failover target rejected psync request");
            return;
        }
    }

    /* If the primary is in an transient error, we should try to PSYNC
     * from scratch later, so go to the error path. This happens when
     * the server is loading the dataset or is not connected with its
     * primary and so forth. */
    if (psync_result == PSYNC_TRY_LATER) {
        syncWithPrimaryHandleError(&conn);
        return;
    }

    /* Note: if PSYNC does not return WAIT_REPLY, it will take care of
     * uninstalling the read handler from the file descriptor. */

    if (psync_result == PSYNC_CONTINUE) {
        serverLog(LL_NOTICE, "PRIMARY <-> REPLICA sync: Primary accepted a Partial Resynchronization.");