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

Code before:
    if (new) {
        node->hostname = sdscpy(node->hostname, new);
    } else if (sdslen(node->hostname) != 0) {
        sdsclear(node->hostname);
    }
    clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG);
}

static void updateAnnouncedHumanNodename(clusterNode *node, char *new) {
    if (new && !strcmp(new, node->human_nodename)) {
        return;
    } else if (!new && (sdslen(node->human_nodename) == 0)) {
        return;
    }

    if (new) {
        node->human_nodename = sdscpy(node->human_nodename, new);
    } else if (sdslen(node->human_nodename) != 0) {
        sdsclear(node->human_nodename);
    }
    clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG);
}


static void updateShardId(clusterNode *node, const char *shard_id) {
    /* Shard id update of replica independently will corrupt the config file. */
    if (shard_id && nodeIsSlave(node) && memcmp(clusterNodeGetMaster(node)->shard_id, shard_id, CLUSTER_NAMELEN) != 0) {
        serverLog(
            LL_NOTICE,
            "Shard id %.40s update request for node id %.40s diverges from existing primary shard id %.40s, rejecting!",
            shard_id, node->name, clusterNodeGetMaster(node)->shard_id);
        return;
    }

    if (shard_id && memcmp(node->shard_id, shard_id, CLUSTER_NAMELEN) != 0) {
        clusterRemoveNodeFromShard(node);
        memcpy(node->shard_id, shard_id, CLUSTER_NAMELEN);
        clusterAddNodeToShard(shard_id, node);
        clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG);
    }
    if (shard_id && myself != node && myself->slaveof == node) {
        if (memcmp(myself->shard_id, shard_id, CLUSTER_NAMELEN) != 0) {
            /* shard-id can diverge right after a rolling upgrade
             * from pre-7.2 releases */
            clusterRemoveNodeFromShard(myself);
            memcpy(myself->shard_id, shard_id, CLUSTER_NAMELEN);
            clusterAddNodeToShard(shard_id, myself);
            clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG | CLUSTER_TODO_FSYNC_CONFIG);
        }
    }
}

Review comment (bug):
I captured your scenario in the comment. 

```suggestion
    /* Ensure replica shard IDs match their primary's to maintain cluster consistency.
     *
     * Shard ID updates must prioritize the primary, then propagate to replicas.
     * This is critical due to the eventual consistency of shard IDs during cluster
     * expansion. New replicas might replicate from a primary before fully
     * synchronizing shard IDs with the rest of the cluster.
     *
     * Without this enforcement, a temporary inconsistency can arise where a
     * replica's shard ID diverges from its primary's. This inconsistency is
     * persisted in the primary's nodes.conf file. While this divergence will
     * eventually resolve, if the primary crashes beforehand, it will enter a
     * crash-restart loop due to the mismatch in its nodes.conf. */
```

Code after:
        goto cleanup;
    }
    latencyEndMonitor(latency);
    latencyAddSampleIfNeeded("cluster-config-rename", latency);

    if (do_fsync) {
        latencyStartMonitor(latency);
        if (fsyncFileDir(server.cluster_configfile) == -1) {
            serverLog(LL_WARNING, "Could not sync cluster config file dir: %s", strerror(errno));
            goto cleanup;
        }
        latencyEndMonitor(latency);
        latencyAddSampleIfNeeded("cluster-config-dir-fsync", latency);
    }
    retval = C_OK; /* If we reached this point, everything is fine. */

cleanup:
    if (fd != -1) {
        latencyStartMonitor(latency);
        close(fd);
        latencyEndMonitor(latency);
        latencyAddSampleIfNeeded("cluster-config-close", latency);
    }
    if (retval == C_ERR) {
        latencyStartMonitor(latency);
        unlink(tmpfilename);
        latencyEndMonitor(latency);
        latencyAddSampleIfNeeded("cluster-config-unlink", latency);
    }
    sdsfree(tmpfilename);
    sdsfree(ci);
    return retval;
}

void clusterSaveConfigOrDie(int do_fsync) {
    if (clusterSaveConfig(do_fsync) == C_ERR) {
        serverLog(LL_WARNING, "Fatal: can't update cluster config file.");
        exit(1);
    }
}

/* Lock the cluster config using flock(), and retain the file descriptor used to
 * acquire the lock so that the file will be locked as long as the process is up.
 *
 * This works because we always update nodes.conf with a new version
 * in-place, reopening the file, and writing to it in place (later adjusting
 * the length with ftruncate()).
 *
 * On success C_OK is returned, otherwise an error is logged and
 * the function returns C_ERR to signal a lock was not acquired. */
int clusterLockConfig(char *filename) {