jihuayu commented on code in PR #389:
URL:
https://github.com/apache/kvrocks-controller/pull/389#discussion_r3225291524
##########
config/config.go:
##########
@@ -47,6 +47,7 @@ type FailOverConfig struct {
// propagates the updated topology. Requires kvrocks to support node
status
// modification (new versions only). Defaults to false for backward
compatibility.
EnableSlaveHAUpdate bool `yaml:"enable_slave_ha_update"`
+ WaitForSync bool `yaml:"wait_for_sync"`
Review Comment:
I'm curious about the significance of wait_for_sync during automatic master
failover. In most cases, when we initiate an automatic switch, the old master
is already inaccessible, meaning the messages we send to it will likely not be
received. Could you explain your reasoning for this?
##########
store/cluster_shard.go:
##########
@@ -239,19 +355,45 @@ func (shard *Shard) promoteNewMaster(ctx context.Context,
masterNodeID, preferre
}
}
if oldMasterNodeIndex == -1 {
- return "", consts.ErrOldMasterNodeNotFound
+ return nil, nil, consts.ErrOldMasterNodeNotFound
}
if masterNodeID != "" && shard.Nodes[oldMasterNodeIndex].ID() !=
masterNodeID {
- return "", consts.ErrNodeIsNotMaster
+ return nil, nil, consts.ErrNodeIsNotMaster
}
newMasterNodeIndex := shard.getNewMasterNodeIndex(ctx,
oldMasterNodeIndex, preferredNodeID)
if newMasterNodeIndex == -1 {
- return "", consts.ErrShardNoMatchNewMaster
+ return nil, nil, consts.ErrShardNoMatchNewMaster
}
+
+ oldMaster := shard.Nodes[oldMasterNodeIndex]
+ newMaster := shard.Nodes[newMasterNodeIndex]
+
+ if opts.WaitForSync {
+ if opts.PauseDuration <= opts.SyncTimeout {
+ return nil, nil, fmt.Errorf("PauseDuration (%v) must be
greater than SyncTimeout (%v)", opts.PauseDuration, opts.SyncTimeout)
+ }
+ if err = oldMaster.PauseClient(ctx, opts.PauseDuration); err !=
nil {
+ return nil, nil, fmt.Errorf("CLIENT PAUSE failed: %w",
err)
+ }
+ defer func() {
+ if err != nil {
+ _ = oldMaster.UnpauseClient(ctx)
+ }
+ }()
+
+ syncErr := shard.waitForReplicationSync(ctx, oldMaster,
newMaster, opts)
Review Comment:
It seems all errors will be ForceOnTimeout
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
