Skip to content
This repository has been archived by the owner on Aug 9, 2024. It is now read-only.

Read-replica support #5

Merged
merged 9 commits into from
Nov 20, 2019
Merged

Read-replica support #5

merged 9 commits into from
Nov 20, 2019

Conversation

smlx
Copy link
Member

@smlx smlx commented Oct 25, 2019

Let's try this again.

Add read replica support to the provisioner. This time the change is not breaking - if the readreplica secret is missing, then:

  • the readreplica service will not be provisioned in the target namespace, but the regular service will be.
  • the DB_READREPLICA_HOST field in the secret will be empty.

Also update README and add notes on development environment, along with scripts and a basic test suite.

@steveworley
Copy link

It looks like we'll be adding a single host to the mariadb_readreplica_hostname, are we expecting that we will only surface one replica via this environment variable? If we had access to multiple readers how would this come back from the DBaaS controller?

For example - Drupal can be configured with n number of replicas and it will send queries to each in a round-robin format. The configuration we'd be looking to use would look like

$databases['default']['replica'][] = [
  'driver' => 'mysql',
  'database' => 'drupaldb1',
  'username' => 'username',
  'password' => 'secret',
  'host' => 'dbserver1',
];

@smlx
Copy link
Member Author

smlx commented Nov 7, 2019

Drupal can be configured with n number of replicas and it will send queries to each in a round-robin format.

Very interesting, I didn't know that!

You're right, this PR doesn't address the multiple-read-replica scenario. I don't think it's worth bundling that into this PR because it's big enough as is. But maybe it makes sense to add a numerical suffix to the environment variable so that multiple read-replicas can be added in future.

e.g. make the variable in this PR DB_READREPLICA_HOST_0, then in future when multiple read-replica support is added we can have DB_READREPLICA_HOST_1, DB_READREPLICA_HOST_2 etc.

@smlx
Copy link
Member Author

smlx commented Nov 8, 2019

...or better, use DB_READREPLICA_HOSTS, and make it a list of space-separated hostnames.

Drupal can make use of multiple read-replicas. This APB doesn't address
that possiblity, but it should in future, so pluralise the interfacing
environment variable so that multiple space-separated hosts can be
defined.
@smlx
Copy link
Member Author

smlx commented Nov 8, 2019

This is ready for review, but please don't merge/deploy until next week to avoid any issues over the weekend.

@smlx
Copy link
Member Author

smlx commented Nov 11, 2019

This is now ready for review.

Copy link

@thom8 thom8 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@shreddedbacon shreddedbacon left a comment

Choose a reason for hiding this comment

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

LGTM

@smlx smlx merged commit 120030a into master Nov 20, 2019
@smlx smlx deleted the read-only-replica branch November 20, 2019 07:17
steveworley added a commit to govCMS/lagoon that referenced this pull request Jan 20, 2020
steveworley added a commit to govCMS/lagoon that referenced this pull request May 5, 2020
* # This is a combination of 3 commits.
# This is the 1st commit message:

Add read replica access.

Updates govcms-deploy to use the read replica for
Adds database config for the read replica
Introduces MARIADB_HOST_READREPLICA environment variable to control the mysql host.

# This is the commit message #2:

Add replicas to the default connection.
# This is the commit message #3:

Update .docker/images/govcms8/scripts/govcms-deploy

* Add read replica access.

Updates govcms-deploy to use the read replica for
Adds database config for the read replica
Introduces MARIADB_HOST_READREPLICA environment variable to control the mysql host.

Add replicas to the default connection.

Update .docker/images/govcms8/scripts/govcms-deploy

Code review updates.

- Return early if host is unavailable.
- Updated db_conf settings for reusability.
- Redirect show tables to stderr.
- Reverted alias change.
- Added output when script is using the reader.

Update the hosts usage.

- Lagoon has updated to use a comma separated list of host names.

Add a condition to make sure trim didn't remove empty.

Update govcms-deploy to check HOSTS.

* Update to split on space as per PR.

- amazeeio/dbaas-mariadb-apb#5 (comment)

* Fixed added replicas in databases array. (#151)

Co-authored-by: Alex Skrypnyk <alex@integratedexperts.com>
Co-authored-by: Alex Skrypnyk <alex.designworks@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants