Skip to content
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

Allow replica-priority value of -1 #8437

Closed
wants to merge 1 commit into from

Conversation

fatpat
Copy link
Contributor

@fatpat fatpat commented Feb 2, 2021

The sentinel replicas <master> command will not expose replicas with a priority of -1.

As with a value of 0, the replica is marked as not able to perform the role of master.

The goal of adding a value (-1) to the config setting replica-priority is to allow ghost replicas. The replica is in the cluster, synchronize with its master, can never be promoted to master and is not exposed to sentinel clients. This way, it is acting as a live backup or living ghost.

The initial use case was the following:

  • we have a 3 nodes redis cluster, with 1 master and 2 replicas
  • we have a 3 nodes sentinel cluster
  • clients ask sentinel which redis is the master for writes
  • clients ask sentinel which redis are the replicas for reads
  • we have real time high load writes to the masters
  • we have real time high load reads from replicas
  • we need async, low frequency, heavy read load using complex and CPU consuming LUA scripts.
  • we want to ensure the async load will not impact performances on the replicas from which the LUA scripts are running
  • we want another replica to be replicated (#captainObvious) but we don't want it to be master nor to accept requests from normal clients
  • so we add the -1 value to replica-priority

this code is running in production on a 6.0.10 cluster and we are happy with it.

@cdelgehier
Copy link

👍

@fatpat fatpat force-pushed the replica_priority_-1 branch 3 times, most recently from 584ff3f to 951b576 Compare February 2, 2021 09:52
The 'sentinel replicas <master>' command will ignore replicas with priority of -1.

As the value 0, the replica is marked as not able to perform the role of master.
@itamarhaber
Copy link
Member

Hello @fatpat - sounds like a nice feature to have. I wonder if any of the Sentinel users that are monitoring this project out there could provide additional feedback about it.

@fatpat
Copy link
Contributor Author

fatpat commented Mar 4, 2021

Hello @fatpat - sounds like a nice feature to have. I wonder if any of the Sentinel users that are monitoring this project out there could provide additional feedback about it.

no one ?
how can we go ahead with this PR ?

@itamarhaber
Copy link
Member

Actually not "no one", but only a couple of upvotes. Getting feedback is a known hard problem.

@yossigo your thoughts on the FR/PR?

@yossigo
Copy link
Member

yossigo commented Mar 7, 2021

@fatpat thanks, this looks interesting.

Do we really want to combine the fact that the replica is hidden from the fact it can't be promoted? Not sure if use cases for that exist and justify it, but at least in theory this could be done with a dedicated flag not to impose this limitation.

Will still be happy to get more user feedback, @hwware any thoughts about this?

@hwware
Copy link
Collaborator

hwware commented Mar 11, 2021

@yossigo I would also vote for a separate flags for the hidden replicas, this will make the config to be more specific. for the relationship of hidden replica which cannot be promoted, we can wait for more use cases on this. However, even this fact is true, we can still check in sentinelSelectSlave to exclude replicas with this flag.

@yossigo
Copy link
Member

yossigo commented Mar 11, 2021

@fatpat Does the above make sense to you?

@fatpat
Copy link
Contributor Author

fatpat commented Mar 11, 2021

@fatpat Does the above make sense to you?

yes it makes sense to me. How would you name this flag ?

  • hidden-replica yes|no
  • replica-hidden yes|no
  • replica-ghost yes|no
  • replica-announced yes|no

I would prefer replica-xxxx to be consistent with other flags like replica-read-only or replica-priority.

@yossigo
Copy link
Member

yossigo commented Mar 11, 2021

@fatpat to me replica-announced seems the most appropriate.

@fatpat
Copy link
Contributor Author

fatpat commented Mar 15, 2021

replaced by #8653

@fatpat fatpat closed this Mar 15, 2021
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.

None yet

5 participants