Skip to content

Replace deprecated rpl_semi_sync_master/slave with rpl_semi_sync_sour…#892

Draft
shunki-fujita wants to merge 3 commits into
mainfrom
issue-847
Draft

Replace deprecated rpl_semi_sync_master/slave with rpl_semi_sync_sour…#892
shunki-fujita wants to merge 3 commits into
mainfrom
issue-847

Conversation

@shunki-fujita

@shunki-fujita shunki-fujita commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

…ce/replica

Migrate semi-synchronous replication from the deprecated master/slave plugins to the new source/replica plugins for MySQL 9.x compatibility.

Key changes:

  • Add SemiSyncNames type and DetectSemiSyncNames() to dynamically detect which semi-sync plugin is installed via information_schema.plugins
  • Make all semi-sync variable references dynamic (SET GLOBAL, SELECT) based on the detected plugin type
  • Rename struct fields: SemiSyncMasterEnabled -> SemiSyncSourceEnabled, SemiSyncSlaveEnabled -> SemiSyncReplicaEnabled, WaitForSlaveCount -> WaitForReplicaCount
  • Update test helpers to install new plugin names

Existing clusters with legacy plugins continue to work because the controller detects the installed plugin at runtime. Migration happens when moco-agent re-initializes plugins on Pod restart.

Ref: #847 cybozu-go/moco-agent#117

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request migrates MOCO's semi-synchronous replication from deprecated MySQL master/slave plugin terminology to the new source/replica terminology for MySQL 9.x compatibility. The changes introduce runtime plugin detection to maintain backward compatibility with existing clusters using legacy plugins while supporting new installations with modern plugin names.

Changes:

  • Added SemiSyncNames type and DetectSemiSyncNames() function to dynamically detect installed semi-sync plugin variant
  • Refactored all semi-sync variable references to use dynamic naming based on detected plugin type
  • Renamed struct fields to use source/replica terminology (e.g., SemiSyncMasterEnabledSemiSyncSourceEnabled)

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/dbop/semisync.go New file implementing plugin detection logic and name mappings for both legacy and new plugin variants
pkg/dbop/types.go Renamed GlobalVariables and GlobalStatus struct fields to source/replica terminology with aliased db tags
pkg/dbop/operator.go Integrated plugin detection during operator initialization with fallback to new names on error
pkg/dbop/status.go Refactored queries to dynamically build SQL using detected plugin variable names
pkg/dbop/replication.go Updated ConfigureReplica and ConfigurePrimary to use dynamic variable names via fmt.Sprintf
pkg/dbop/test_util.go Updated test helper to install new plugin names and added detection call in test operator creation
pkg/dbop/status_test.go Updated test assertions to use new struct field names
pkg/dbop/replication_test.go Updated test assertions to use new struct field names
clustering/status.go Updated to use renamed GlobalStatus field
clustering/operations.go Updated to use renamed GlobalVariables fields
clustering/mock_test.go Updated mock implementation to use new field names
clustering/manager_test.go Updated test assertions to use new field names

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/dbop/operator.go Outdated
Comment thread pkg/dbop/semisync.go Outdated
Comment thread pkg/dbop/semisync.go Outdated
…ce/replica

Migrate semi-synchronous replication from the deprecated master/slave
plugins to the new source/replica plugins for MySQL 9.x compatibility.

Key changes:
- Add SemiSyncNames type and DetectSemiSyncNames() to dynamically detect
  which semi-sync plugin is installed via information_schema.plugins
- Make all semi-sync variable references dynamic (SET GLOBAL, SELECT)
  based on the detected plugin type
- Rename struct fields: SemiSyncMasterEnabled -> SemiSyncSourceEnabled,
  SemiSyncSlaveEnabled -> SemiSyncReplicaEnabled,
  WaitForSlaveCount -> WaitForReplicaCount
- Update test helpers to install new plugin names

Existing clusters with legacy plugins continue to work because the
controller detects the installed plugin at runtime. Migration happens
when moco-agent re-initializes plugins on Pod restart.

Ref: #847

Signed-off-by: shunki-fujita <shunki-fujita@cybozu.co.jp>
…lugins

Propagate the error from DetectSemiSyncNames in the operator factory and
close the connection on failure, so the reconciler retries instead of
silently falling back to NewSemiSyncNames when information_schema returns
an error. The silent fallback would target nonexistent variables on a
legacy-plugin cluster and turn a transient query failure into persistent
SET GLOBAL failures.

Change SemiSyncNames returns from *SemiSyncNames to SemiSyncNames so the
operator field cannot be nil and callers cannot accidentally mutate the
package-level constants through the returned pointer.

Emit a warning via logr when DetectSemiSyncNames finds a semi-sync plugin
that is registered but not ACTIVE (DISABLED / INACTIVE / DELETED). The
system variables for non-ACTIVE plugins are not exposed by MySQL, so
later SET/SELECT against them will fail; surfacing the state at detection
time aids debugging. The warning is intentionally not emitted when both
plugins are absent, which is the normal state of a freshly-created
instance whose moco-agent has not yet installed the plugins.

Signed-off-by: shunki-fujita <shunki-fujita@cybozu.co.jp>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comment thread pkg/dbop/operator.go Outdated
Treat DetectSemiSyncNames errors the same way as Resolver failures: log
and return a NopOperator instead of propagating. GatherStatus aborts the
whole reconcile cycle on a New() error, while per-instance GetStatus
errors are tolerated, so a single replica being briefly unreachable
during pod restart, network blips, or failover should not stop the
reconciler from making progress on the rest of the cluster.

Signed-off-by: shunki-fujita <shunki-fujita@cybozu.co.jp>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants