-
Notifications
You must be signed in to change notification settings - Fork 422
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
Support new regional HA MySQL #99
Support new regional HA MySQL #99
Conversation
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
It would be very helpful if someone could expedite review for this feature. We are unable to use Terraform to manage Cloud SQL instances with high availability until this feature is released. |
modules/mysql/variables.tf
Outdated
variable "availability_type" { | ||
description = "The availability type for the master instance. Can be either `ZONAL` or `REGIONAL`." | ||
type = string | ||
default = "ZONAL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend to set default to REGIONAL as it is new default value and in July it will be even not possible to change it to ZONAL, also this info probably worth adding to description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either REGIONAL or null but not ZONAL. Both values (regional and null) will be correct in the future as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in c247027 by setting to REGIONAL
, description also updated to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that null is the value to create non-ha CloudSQL
Signed-off-by: Dev <devan@ingresso.co.uk>
Signed-off-by: Dev <devan@ingresso.co.uk>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach LGTM, will merge once tests pass.
We can leave the legacy failover stuff for now and optionally remove it in a subsequent release.
@@ -22,6 +22,10 @@ locals { | |||
enabled = var.ip_configuration | |||
disabled = {} | |||
} | |||
|
|||
// HA method using REGIONAL availability_type requires binary logs to be enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it true that REGIONAL
availability_type
requires binary logs? From poking around the docs it seems like binary logs are only required if you want to use a read replica or if you want point in time recovery?
These settings determine whether automated backups are performed and whether binary logging is enabled or not. Both options add a small performance cost, but are required for the creation of replicas and clones, and for point-in-time recovery.
https://cloud.google.com/sql/docs/mysql/instance-settings#backups-and-binary-logging-2ndgen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It fails without bin logs enabled, the GCP docs mention this for enabling HA
https://cloud.google.com/sql/docs/mysql/configure-ha#console
https://cloud.google.com/sql/docs/mysql/configure-ha#gcloud
Leave the Automate backups and Enable point-in-time recovery checkboxes enabled.
Which requires bin logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very interesting, thanks for those links. I've recreated the same thing for myself with gcloud, the REST API, and the GCP UI.
I'll follow up with a GCP Support ticket to understand if they'll offer more flexibility in the future. Thanks for writing up the PR!
Closes #71
https://cloud.google.com/sql/docs/mysql/high-availability#HA-configuration
WIP still need to test/update docs
I can also remove the legacy replica stuff for #97 but that does mean a breaking release/2 step migration path (Updating an instance from legacy to current high availability). Thoughts @morgante ?