or maybe in CLUSTER REPLICATE, we should call a clusterDelNodeSlots(myself) if we find out myself is a replica.
i will try to add a test to cover it later

A replica in cluster should not possess any slot.

I think we should forbid ADDSLOT and ADDSLOTRANGE to execute on a replica, and check whether "myself" has any slot in CLUSTER REPLICATE, instead of deleting slots from a node which should not have any.

A replica in cluster should not possess any slot.
I think we should forbid ADDSLOT and ADDSLOTRANGE to execute on a replica, and check whether "myself" has any slot in CLUSTER REPLICATE, instead of deleting slots from a node which should not have any.

yeah, i agree with this. I checked it and didn't see any restrictions on replicas, and since the code been there forever, i plan to keep it for now. anyway, i am ok with both, let's see what other people say

A replica in cluster should not possess any slot.

I think we should forbid ADDSLOT and ADDSLOTRANGE to execute on a replica, and check whether "myself" has any slot in CLUSTER REPLICATE, instead of deleting slots from a node which should not have any.

+1

A replica in cluster should not possess any slot.

Yeah, replicas are by design not supposed to own any slots. I would rather update the judgements to prevent the replica from trying to take ownership of slots. I'm fairly sure it was just an oversight at the time, the code has always been fairly fragile.

and check whether "myself" has any slot in CLUSTER REPLICATE

we do this when master:

        /* If the instance is currently a master, it should have no assigned
         * slots nor keys to accept to replicate some other node.
         * Slaves can switch to another master without issues. */
        if (nodeIsMaster(myself) &&
            (myself->numslots != 0 || dictSize(server.db[0].dict) != 0)) {
            addReplyError(c,
                "To set a master the node must be empty and "
                "without assigned slots.");
            return;
        }

now with the new changes, do we need to check the replicas?

@madolson please take another look. IIUC it's not urgent, but there's probably no reason not to fix it one way or the other.

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}