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

Code before:
        data_age = (mstime_t)(server.unixtime - server.repl_down_since) * 1000;
    }

    /* Remove the node timeout from the data age as it is fine that we are
     * disconnected from our primary at least for the time it was down to be
     * flagged as FAIL, that's the baseline. */
    if (data_age > server.cluster_node_timeout) data_age -= server.cluster_node_timeout;

    /* Check if our data is recent enough according to the replica validity
     * factor configured by the user.
     *
     * Check bypassed for manual failovers. */
    if (server.cluster_replica_validity_factor &&
        data_age > (((mstime_t)server.repl_ping_replica_period * 1000) +
                    (server.cluster_node_timeout * server.cluster_replica_validity_factor))) {
        if (!manual_failover) {
            clusterLogCantFailover(CLUSTER_CANT_FAILOVER_DATA_AGE);
            return;
        }
    }

    /* If the previous failover attempt timeout and the retry time has
     * elapsed, we can setup a new one. */
    if (auth_age > auth_retry_time) {
        server.cluster->failover_auth_time = now +
                                             random() % 500; /* Random delay between 0 and 500 milliseconds. */
        server.cluster->failover_auth_count = 0;
        server.cluster->failover_auth_sent = 0;
        server.cluster->failover_auth_rank = clusterGetReplicaRank();
        /* We add another delay that is proportional to the replica rank.
         * Specifically 1 second * rank. This way replicas that have a probably
         * less updated replication offset, are penalized. */
        server.cluster->failover_auth_time += server.cluster->failover_auth_rank * 1000;
        /* We add another delay that is proportional to the failed primary rank.
         * Specifically 0.5 second * rank. This way those failed primaries will be
         * elected in rank to avoid the vote conflicts. */
        server.cluster->failover_failed_primary_rank = clusterGetFailedPrimaryRank();
        server.cluster->failover_auth_time += server.cluster->failover_failed_primary_rank * 500;
        /* However if this is a manual failover, no delay is needed. */
        if (server.cluster->mf_end) {
            server.cluster->failover_auth_time = now;
            server.cluster->failover_auth_rank = 0;
            /* Reset auth_age since it is outdated now and we can bypass the auth_timeout
             * check in the next state and start the election ASAP. */
            auth_age = 0;
        } else if (server.cluster->failover_auth_time == now) {
            /* If we happen to initiate a failover immediately, reset auth_age since it is
             * outdated now and we can bypass the auth_timeout check in the next state and
             * start the election ASAP. */
            auth_age = 0;
        }

Review comment (bug):
So it seem that the main point of the fixed delay is not to let FAIL propagate, but to let the replicas converge on rank. The code comment is misleading.

The other (safer) idea that doesn't change any logic is to just make these 500 be relative to the node time, as discussed in #2023. The delay we need depends on speed, just like node timeout.

If we replace 500 with `node_timeout / 30`, we keep 500 for the default node timeout 15000 but use shorter delay for if node timeout is smaller.

| node timeout | node_timeout / 30 |
|--------------|-------------------|
| 15000        | 500               |
| 6000         | 200               |
| 3000         | 100               |
| 2000         | 66                |
| 1500         | 50                |

For node timeout 2000, we have observed the total time from killing a node until successful failover is usually 3.0-3.5 seconds. With this shorter delay, we can reduce it to 2.5-2.6 seconds and save almost one second for the highest random() value.

```c
        int delay = min(node_timeout / 30, 500);
        server.cluster->failover_auth_time = now +
                                             delay +           /* Fixed delay to converge on rank. */
                                             random() % delay; /* Random delay between 0 and delay. */
```


Code after:
        data_age = (mstime_t)(server.unixtime - server.primary->last_interaction) * 1000;
    } else {
        data_age = (mstime_t)(server.unixtime - server.repl_down_since) * 1000;
    }

    /* Remove the node timeout from the data age as it is fine that we are
     * disconnected from our primary at least for the time it was down to be
     * flagged as FAIL, that's the baseline. */
    if (data_age > server.cluster_node_timeout) data_age -= server.cluster_node_timeout;

    /* Check if our data is recent enough according to the replica validity
     * factor configured by the user.
     *
     * Check bypassed for manual failovers. */
    if (server.cluster_replica_validity_factor &&
        data_age > (((mstime_t)server.repl_ping_replica_period * 1000) +
                    (server.cluster_node_timeout * server.cluster_replica_validity_factor))) {
        if (!manual_failover) {
            clusterLogCantFailover(CLUSTER_CANT_FAILOVER_DATA_AGE);
            return;
        }
    }

    /* If the previous failover attempt timeout and the retry time has
     * elapsed, we can setup a new one. */
    if (auth_age > auth_retry_time) {
        server.cluster->failover_auth_time = now +
                                             500 +           /* Fixed delay of 500 milliseconds, let FAIL msg propagate. */
                                             random() % 500; /* Random delay between 0 and 500 milliseconds. */
        server.cluster->failover_auth_count = 0;
        server.cluster->failover_auth_sent = 0;
        server.cluster->failover_auth_rank = clusterGetReplicaRank();
        /* We add another delay that is proportional to the replica rank.
         * Specifically 1 second * rank. This way replicas that have a probably
         * less updated replication offset, are penalized. */
        server.cluster->failover_auth_time += server.cluster->failover_auth_rank * 1000;
        /* We add another delay that is proportional to the failed primary rank.
         * Specifically 0.5 second * rank. This way those failed primaries will be
         * elected in rank to avoid the vote conflicts. */
        server.cluster->failover_failed_primary_rank = clusterGetFailedPrimaryRank();
        server.cluster->failover_auth_time += server.cluster->failover_failed_primary_rank * 500;
        /* However if this is a manual failover, no delay is needed. */
        if (server.cluster->mf_end) {
            server.cluster->failover_auth_time = now;
            server.cluster->failover_auth_rank = 0;
            server.cluster->failover_failed_primary_rank = 0;
            /* Reset auth_age since it is outdated now and we can bypass the auth_timeout
             * check in the next state and start the election ASAP. */
            auth_age = 0;
        }
        serverLog(LL_NOTICE,