server icon indicating copy to clipboard operation
server copied to clipboard

[MDEV-34009] Introduce server-initiated instant failover mechanism (and TLS fixes)

Open dlenski opened this issue 1 year ago • 12 comments

This PR is a replacement for https://github.com/MariaDB/server/pull/2681, which proposed a very similar feature under the name of "server redirect"

Upstream developers chose a different approach (see https://github.com/MariaDB/server/pull/1472 and https://github.com/mariadb-corporation/mariadb-connector-c/pull/137) for that feature. See MDEV-15935 ("connection redirection") for further discussion.

I maintain that a simple and obligatory server-directed failover mechanism will be a very useful feature for many users, so I'm reintroducing it here under the alternative name of "instant failover."

Description

This PR proposes a new feature in the MariaDB client-server protocol: a server-initiated failover which can redirect new client connections to an alternate endpoint at the application layer.

It also provides an initial server-side implementation of that feature; the corresponding client-side implementation is in https://github.com/mariadb-corporation/mariadb-connector-c/pull/245.

We want the MariaDB server to be able to tell clients connecting to it, “Sorry, this server is unavailable. Connect to an alternate server instead.” This mechanism is inspired by HTTP 302 (“temporary redirect”) mechanism familiar to all developers of web applications, and is intended to have similar semantics and security properties, since these have now been widely deployed and tested for decades.

How can this PR be tested?

Automated testing

  • [X] The newly-added main.instant_failover MTR test functionally verifies the instant failover mechanism.
  • [x] Pre-existing MTR tests are updated and passing.

Manual testing

  • Build the server from this PR, along with the updated client from https://github.com/mariadb-corporation/mariadb-connector-c/pull/245 (included in this PR as a submodule change).
  • Start the server with sql/mariadbd --instant-failover-mode=ON --instant-failover-target='some-other-mariadb.server.com:3306' --port=3306 --extra-port=3307 --socket=/tmp/mysql.sock
  • Connect with the updated client and observe the failover/redirection behavior, including the fact that connections to the EXTRA_PORT and to the Unix socket are not redirected:
    • client/mariadb --host 127.0.0.1 --port=3306 -uUSERNAME -pPASSWORD → redirected to INSTANT_FAILOVER_TARGET - client/mariadb --host 127.0.0.1 --port=3307 -uUSERNAME -pPASSWORDnot redirected (EXTRA PORT)
    • client/mariadb --socket=/tmp/mysql.sock -uUSERNAME -pPASSWORDnot redirected (local/non-network-based connection)

Basing the PR against the correct MariaDB version

  • [x] This is a new feature and the PR is based against the latest MariaDB development branch, 11.5.

PR quality check

dlenski avatar Apr 25 '24 20:04 dlenski

CLA assistant check
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.

CLAassistant avatar Apr 25 '24 20:04 CLAassistant

Hi @dlenski.

Some thoughts...

  • There appear to be TLS related fixes in this unrelated to the topic at hand, these should probably be in a separate Jira / PR.
  • What protections are there from a MITM fake server from issuing a redirect?
  • Is there really a use case for this? I would imagine most people would use a proxy or do it client side?
  • It should probably be attached to a new Jira, it is difficult to get it reviewed / tested under a closed Jira.

LinuxJedi avatar Apr 26 '24 11:04 LinuxJedi

Hi @dlenski.

Some thoughts...

  • There appear to be TLS related fixes in this unrelated to the topic at hand, these should probably be in a separate Jira / PR.
  • What protections are there from a MITM fake server from issuing a redirect?

These first two bullet points are closely related… in fact, essentially all of my TLS-related PRs (the accepted ones, the stalled ones, and the closed ones) grew out of TLS-related discoveries made during the initial implementation of this feature. :grimacing:

  • Is there really a use case for this?

From my vantage point, there is one very clear use case for this feature as implemented here (gracefully redirecting clients to an alternate server in preparation to shutdown this one for maintenance), and it's easily extensible for other use cases such as load balancing (redirecting some-but-not-all new clients away).

In this comment, @ottok nicely explains how this solution here is a very simple one for redirecting clients, and extremely time-tested in other kinds of application protocols including HTTP in particular.

In this comment, I expressed my confusion about why an alternative solution was chosen for MDEV-15935, given that "As you noted yourself on the mailing list, you don't know of any actual use cases for advisory-only redirection. By contrast, we are aware of one major use case for "forceful" redirection, graceful server shutdown, as discussed here and on the mailing list."

I would imagine most people would use a proxy or do it client side?

I'm not sure what "do it client side" means. This PR does indeed involve a change in Connector/C, so that the client can automatically and seamlessly .

As for "use a proxy [server]", that means adding another layer of complexity.

  • It should probably be attached to a new Jira, it is difficult to get it reviewed / tested under a closed Jira.

Ack.

Created MDEV-34009 and rewrote the PR and commits to reference this new JIRA.

dlenski avatar Apr 26 '24 20:04 dlenski

Hi @dlenski. Some thoughts...

  • There appear to be TLS related fixes in this unrelated to the topic at hand, these should probably be in a separate Jira / PR.
  • What protections are there from a MITM fake server from issuing a redirect?

These first two bullet points are closely related… in fact, essentially all of my TLS-related PRs (the accepted ones, the stalled ones, and the closed ones) grew out of TLS-related discoveries made during the initial implementation of this feature. 😬

My intention was that these two aren't quite related. If you do not have TLS enabled, for whatever reason, this seems vulnerable to me. Or can this feature only be used when TLS is enabled? (I didn't spot that).

As for the MDEV-33822 fix, if that is a bug fix for a 11.4 thing, it should probably go to 11.4, as this is an LTS release there will be more versions of it. It shouldn't be in this PR. Especially since new features won't end up landing in 11.5 any more.

There is at least one other commit in this PR that will conflict soon as it appears to fix something that hasn't merged all the way up yet.

  • Is there really a use case for this?

From my vantage point, there is one very clear use case for this feature as implemented here (gracefully redirecting clients to an alternate server in preparation to shutdown this one for maintenance), and it's easily extensible for other use cases such as load balancing (redirecting some-but-not-all new clients away).

In this comment, @ottok nicely explains how this solution here is a very simple one for redirecting clients, and extremely time-tested in other kinds of application protocols including HTTP in particular.

In this comment, I expressed my confusion about why an alternative solution was chosen for MDEV-15935, given that "As you noted yourself on the mailing list, you don't know of any actual use cases for advisory-only redirection. By contrast, we are aware of one major use case for "forceful" redirection, graceful server shutdown, as discussed here and on the mailing list."

OK, I'll rephrase my point here. Why wouldn't you issue it from the proxy? In those use cases you normally have a proxy so that the web application would always connect to the correct DB server anyway.

Is there citation for where users are asking for this explicitly to be in the DB server? Or are other DB servers doing this?

I would imagine most people would use a proxy or do it client side?

I'm not sure what "do it client side" means. This PR does indeed involve a change in Connector/C, so that the client can automatically and seamlessly .

I meant in the end user application.

As for "use a proxy [server]", that means adding another layer of complexity.

  • It should probably be attached to a new Jira, it is difficult to get it reviewed / tested under a closed Jira.

Ack.

Created MDEV-34009 and rewrote the PR and commits to reference this new JIRA.

Many thanks.

LinuxJedi avatar Apr 29 '24 09:04 LinuxJedi

Thanks for the review @LinuxJedi!

My intention was that these two aren't quite related. If you do not have TLS enabled, for whatever reason, this seems vulnerable to me. Or can this feature only be used when TLS is enabled? (I didn't spot that).

No it does not, but it does ensure that if TLS is used, the redirect happens after TLS connection has been setup.

MariaDB already has variable redirect_url which can work without TLS encryption, so this PR does not change the situation in that regard. If you are connecting without TLS in your client all bets are off anyway.

As for the MDEV-33822 fix, if that is a bug fix for a 11.4 thing, it should probably go to 11.4

Feel free to git cherry-pick that commit to another branch. This PR also includes other commits that have already been submitted separately, or which could be included separately but haven't been submitted separately due to low success rate for those previous separate submissions.

Is there citation for where users are asking for this explicitly to be in the DB server? Or are other DB servers doing this?

Yes, anyone not using a proxy setup or special clustering software would benefit from this. We are not aware of other DB servers doing this (yet), but a lot of HTTP servers are doing this.

ottok avatar Apr 29 '24 15:04 ottok

My intention was that these two aren't quite related. If you do not have TLS enabled, for whatever reason, this seems vulnerable to me. Or can this feature only be used when TLS is enabled? (I didn't spot that).

No it does not, but it does ensure that if TLS is used, the redirect happens after TLS connection has been setup.

Exactly.

And because https://jira.mariadb.org/browse/CONC-648 is now fixed in Connector/C (in https://github.com/mariadb-corporation/mariadb-connector-c/commit/ebcb9ec), a MariaDB Connector/C client using TLS will not accept the failover/redirect packet until after authentication and TLS setup is complete.

dlenski avatar Apr 29 '24 22:04 dlenski

I've pared down this PR (and the corresponding client-side PR) to remove all of the commits which address pre-existing TLS issues which were discovered or highlighted during its development:

  • https://github.com/MariaDB/server/pull/3231
  • https://github.com/mariadb-corporation/mariadb-connector-c/pull/246

In other words, this :point_up: pair of PRs only introduces the instant failover feature and adds tests for it and doesn't touch anything else.

dlenski avatar Apr 29 '24 23:04 dlenski

Is there citation for where users are asking for this explicitly to be in the DB server? Or are other DB servers doing this?

Yes, anyone not using a proxy setup or special clustering software would benefit from this. We are not aware of other DB servers doing this (yet), but a lot of HTTP servers are doing this.

But this feature seems at odds with why you would want a redirect. So, if high-availability is a goal, a stack would have multiple DB servers fronted by a proxy which would handle the fail over. Because not all maintenance is planned or happens during working hours. This is why I'm struggling to see a use case.

In HTTP stacks that have similar high-availability requirements, it is typically the proxy that handles it. Sure, PHP's FastCGI or whatever it is today can probably do it, but you would likely do it in NGINX.

LinuxJedi avatar Apr 30 '24 07:04 LinuxJedi

The HTTP 302 redirect is not used in High Availability setups. Usually in HTTP you have multiple IP addresses for one single hostname and then multiple HTTP servers responding at those IP addresses, and various layering of those HTTP servers with reverse proxies etc.

This is not mimicking that at all. This is for instant planned failover just like HTTP 302 redirects, so you can retire one server and replace it with a new one in a simple way.

ottok avatar Apr 30 '24 14:04 ottok

The HTTP 302 redirect is not used in High Availability setups. Usually in HTTP you have multiple IP addresses for one single hostname and then multiple HTTP servers responding at those IP addresses, and various layering of those HTTP servers with reverse proxies etc. This is not mimicking that at all. This is for instant planned failover just like HTTP 302 redirects, so you can retire one server and replace it with a new one in a simple way.

The last sentence of this is the start of what I'm looking for. Now if you can expand that into a full user story / use case in the Jira, we can look into it.

I don't think 302 is a great analogy for this. What you are describing appears to be something above the web server redirecting temporarily from one to the other, a web server under maintenance wouldn't be issuing 302s because you don't know how reliable it will be during the maintenance. It might be a better analogy for something like FederatedX.

But at least I finally understand a bit of your thinking behind it.

LinuxJedi avatar May 01 '24 10:05 LinuxJedi

If you have a connection and are tracking the session_track_system_variables=redirect_url variable you'll get a server instigated redirect notice in the protocol on a result.

It seems server shutdown could force connections to receive a protocol message as a result.

I haven't followed the discussion in numerous places on this. Just looking at solving a use case without new protocol changes.

https://mariadb.com/kb/en/ok_packet/

grooverdan avatar May 02 '24 00:05 grooverdan

I've updated https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/78 to have the latest version of this and I intend to work on this once we have all vector stuff done first.

ottok avatar Jul 08 '24 04:07 ottok