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

Feature Request: provide vtctldclient functions to manipulate Super Read Only. #12880

Open
jfg956 opened this issue Apr 12, 2023 · 7 comments
Open

Comments

@jfg956
Copy link
Contributor

jfg956 commented Apr 12, 2023

Feature Description

Hi, I am opening an issue as requested in {1} thread. Below mostly a copy of this thread.
{1}: https://vitess.slack.com/archives/C0PQY0PTK/p1681311693851529

In the last Vitess Monthly Meeting, it was mentioned there was work to leverage SUPER_READ_ONLY (sro) in vttablet. At the time, I mentioned I had feedback, it is in this thread.

PR for this work: Initialize Tablet with super_read_only mode / #12206

In above PR, I understand vtctlclient allows to return the sro status, I think there should be other vtctlclient functions related to this:

  • unset sro: set sro to false, erroring if sro is already false,
  • set sro: set sro to true, erroring if sro is already true, and erroring on a primary (unless a force option is given).

The reason I think the above functions are needed is that there are conditions where we want to write on a replica, so Vitess should allow that and provide "function" to do that in.a safe way. Manually setting sro to false via MySQL is risky as it introduces a race condition with other Vitess functions (like Reparent). I have experience in my environment where a script - setting sro to false, doing some work, and then setting it back to true to true - was running while a Reparent happened, and set a primary as Read-Only --> not fun !

Thanks in advance for considering this feature request.

Use Case(s)

See Feature Request Description.

Basically, allow manipulation Super Read Only in a "Vitess Safe Way".

@jfg956 jfg956 added Needs Triage This issue needs to be correctly labelled and triaged Type: Feature labels Apr 12, 2023
@ajm188
Copy link
Contributor

ajm188 commented Apr 12, 2023

I'm amending this to vtctldclient, since that's where all new behavior should go.

@ajm188 ajm188 changed the title Feature Request: provide vtctlclient functions to manipulate Super Read Only. Feature Request: provide vtctldclient functions to manipulate Super Read Only. Apr 12, 2023
@ajm188 ajm188 added Component: Cluster management and removed Needs Triage This issue needs to be correctly labelled and triaged labels Apr 12, 2023
@makmanalp
Copy link

To clarify a bit more on the use case of why anyone would want to write to a replica, we do it to store non-replicated, replica-specific metadata and state information, as well as performance stats etc.

@rsajwani rsajwani self-assigned this Apr 17, 2023
@rsajwani
Copy link
Contributor

@jfg956, yeah, I don't see any issue providing safe way to manipulate 'Super Read Only'. Would you like to contribute to this? Give I am working on other work items and it might take me sometime before I start working on it.

@jfg956
Copy link
Contributor Author

jfg956 commented Apr 19, 2023

I will not have time to contribute time to this.

@deepthi
Copy link
Member

deepthi commented Apr 19, 2023

I'm not convinced that we should allow people to violate the safety being provided by Vitess through an API like this. If there are others besides y'all who need this feature we can consider including it. Otherwise you will need to do it in your fork versus on the mainline development of the project.
@mattlord what are your thoughts?

@mattlord
Copy link
Contributor

I tend to agree with @deepthi here. Making it easy to manipulate would go against the guarantees we're after by enforcing its use.

At the same time I do understand the MySQL instance level use case. @jfg956 Is the issue that you want a way to disable/enable it across N mysqld instances vs having to connect to each mysqld to enable/disable it? If we do make it easy to disable, then I think we'd probably need to make this something that vtorc monitors and repairs -- to ensure that this gets re-enabled in a timely fashion.

@jfg956
Copy link
Contributor Author

jfg956 commented Apr 26, 2023

Is the issue that you want a way to disable/enable it across N mysqld instances vs having to connect to each mysqld to enable/disable it?

I want to disable/enable it on a per instance basis (no need to to it on N instances in a single API call).

I would like us to be realistic and recognied that these is a use-case for writing on replicas, just for "adding and index" as an example. I would not want vtorc to magically repair this as it might "break" a script that is currently running and needing to write to a replica. This should go in monitoring and alerting: a replica not being sro for X minutes should "alert"; not in Vitess "magic" automation.

Making it easy to manipulate would go against the guarantees we're after by enforcing its use.

And not making it easy to manipulate is "negating" (not sure it is the right English world, forgive my French) the valid use-case of writing to replicas. A SUPER user already has "grants" to manipulate this, Vitess should "embrace" this and not try to "enforce" an "opinion / religion" that writing to replicas is something that should never happen.

As described in this feature request, having both Vitess and a SUPER user "manipulating" sro introduces race-conditions that can have very bad consequences (a script disabling and enabling sro running on a replica while it is being reparented to and ending up setting a primary read-only).

And this is why I still think Vitess should provide APIs for manipulating sro in a "safe" way.

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

No branches or pull requests

6 participants