Skip to content

chore: allow replicaof in cluster mode when state is TAKEN_OVER #5618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kostasrim
Copy link
Contributor

REPLTAKEOVER in cluster mode does not shut down master. Now a node that is in TAKEN_OVER_STATE (i,e, repltakeover) is allowed to become a replica of another node via the REPLICAOF command.

  • allow REPLICAOF command in TAKEN_OVER server state and only in cluster mode

Resolves #5611

@kostasrim kostasrim requested review from romange and dranikpg August 4, 2025 14:33
@kostasrim kostasrim self-assigned this Aug 4, 2025
Comment on lines +1195 to +1197
bool replicaof_in_cluster = cid->name() == "REPLICAOF" && IsClusterEnabled();
allowed_by_state = dfly_cntx.conn()->IsPrivileged() || (cid->opt_mask() & CO::ADMIN) ||
cid->name() == "PING";
cid->name() == "PING" || replicaof_in_cluster;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just a command that reverts from TAKEN OVER To ACTIVE would've been easier

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean like a proper command in the registry right ?

I thought about it as well, the reason I chose this was to support a direct change from taken over to replica and since the replica flow flushed the datastore anyway it seemed to be simplest one 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though of an ADMIN command and only then I realized that REPLICAOF already is one. Do we really need this condition if we have cid->opt_mask() & CO::ADMIN 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REPLICAOF fails after takeover
3 participants