add improvements for async deployment - #1385
Draft
hors wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves async (Orchestrator-based) deployments by making rollouts less prone to deadlock, adding automated repair for certain broken replication channels after failover, and introducing a policy-driven way to handle errant GTIDs (including optional Orchestrator-assisted reconciliation).
Changes:
- Propagate TLS topology-monitoring settings into the Orchestrator config and extend Orchestrator client support for replication thread state + errant GTIDs operations.
- Improve StatefulSet smart update behavior by proactively deleting unready pods that are still on an outdated controller revision.
- Add async replica repair logic (including configurable errant GTID handling), wire the new policy into CRDs/examples/defaults, and expand unit coverage around backup image validation.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/orchestrator/orchestrator.go | Adds TLS topology-monitoring settings into generated Orchestrator config when SSL secret is configured. |
| pkg/orchestrator/client.go | Extends Orchestrator API model with replication thread states, errant GTID field, and adds gtid-errant-inject-empty endpoint wrapper. |
| pkg/db/replication.go | Ensures replication channel is configured with GET_SOURCE_PUBLIC_KEY=1 during CHANGE REPLICATION SOURCE. |
| pkg/controller/ps/upgrade.go | Avoids rollout deadlock by deleting pods that are unready and still on an outdated revision before waiting for full readiness. |
| pkg/controller/ps/controller.go | Improves ordered deletion logging, adds broken-replica repair and errant GTID handling during replication reconciliation. |
| deploy/cw-bundle.yaml | Updates bundled CRD schema to include errantTransactionsPolicy with default/enum. |
| deploy/crd.yaml | Updates CRD schema to include errantTransactionsPolicy with default/enum. |
| deploy/cr.yaml | Documents the new errantTransactionsPolicy field in the example CR. |
| deploy/bundle.yaml | Updates bundled CRD schema to include errantTransactionsPolicy with default/enum. |
| config/crd/bases/ps.percona.com_perconaservermysqls.yaml | Adds errantTransactionsPolicy to the base CRD schema with default/enum. |
| cmd/internal/db/db.go | Adds GET_SOURCE_PUBLIC_KEY=1 to internal replication channel setup SQL. |
| cmd/example-gen/scripts/lib/ps.sh | Excludes spec.mysql.errantTransactionsPolicy from generated example output. |
| cmd/example-gen/pkg/defaults/manual.go | Sets default ErrantTransactionsPolicy for manual defaults generation. |
| cmd/bootstrap/async/async_replication.go | Prevents cloning over a datadir that already executed GTIDs to avoid silent data loss after failover. |
| api/v1/perconaservermysql_types.go | Introduces ErrantTransactionsPolicy API type and exposes it on MySQLSpec. |
| api/v1/perconaservermysql_types_test.go | Adds coverage for backups-disabled scenarios regarding backup image requirement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.Info("Ensuring oldest mysql node is the primary") | ||
| currentPrimary, err := orchestrator.ClusterPrimary(ctx, r.ClientCmd, orcPod, cr.ClusterHint()) | ||
| if err != nil { | ||
| log.Info("Could not determine current primary from Orchestrator, skipping primary switch", "error", err) |
Comment on lines
+1138
to
+1144
| for _, instance := range clusterInstances { | ||
| if instance.Alias == primary.Alias { | ||
| continue | ||
| } | ||
| if instance.MasterKey.Hostname != primary.Key.Hostname { | ||
| continue | ||
| } |
Comment on lines
+164
to
+168
| log.Printf("Datadir has executed GTIDs (%s), skipping clone to preserve local data", gtidExecuted) | ||
| requireClone = false | ||
|
|
||
| if err := createCloneLock(cloneLock); err != nil { | ||
| return errors.Wrap(err, "create clone lock") |
Collaborator
commit: 459ae65 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability