Merge pull request #5291 from planetscale/fix-reparent-refactor

Fix vtgate_buffer test

go/vt/vttablet/tabletmanager/shard_sync.go

@@ -28,6 +28,7 @@ import (
  topodatapb "vitess.io/vitess/go/vt/proto/topodata"
  "vitess.io/vitess/go/vt/topo"
  "vitess.io/vitess/go/vt/topo/topoproto"
+ "vitess.io/vitess/go/vt/vterrors"
 )
 
 var (
@@ -106,6 +107,7 @@ func (agent *ActionAgent) shardSyncLoop(ctx context.Context) {
  if !topoproto.TabletAliasEqual(masterAlias, tablet.Alias) {
  // Another master has taken over while we still think we're master.
  if err := agent.abortMasterTerm(ctx, masterAlias); err != nil {
+ log.Errorf("Failed to abort master term: %v", err)
  // Start retry timer and go back to sleep.
  retryChan = time.After(*shardSyncRetryDelay)
  continue
@@ -187,25 +189,34 @@ func syncShardMaster(ctx context.Context, ts *topo.Server, tablet *topodatapb.Ta
 // If active reparents are disabled, we don't touch our MySQL.
 // We just directly update our tablet type to REPLICA.
 func (agent *ActionAgent) abortMasterTerm(ctx context.Context, masterAlias *topodatapb.TabletAlias) error {
- ctx, cancel := context.WithTimeout(ctx, *topo.RemoteOperationTimeout)
- defer cancel()
-
  masterAliasStr := topoproto.TabletAliasString(masterAlias)
  log.Warningf("Another tablet (%v) has won master election. Stepping down to REPLICA.", masterAliasStr)
 
  if *mysqlctl.DisableActiveReparents {
  // Don't touch anything at the MySQL level. Just update tablet state.
  log.Infof("Active reparents are disabled; updating tablet state only.")
- return agent.ChangeType(ctx, topodatapb.TabletType_REPLICA)
+ changeTypeCtx, cancel := context.WithTimeout(ctx, *topo.RemoteOperationTimeout)
+ defer cancel()
+ if err := agent.ChangeType(changeTypeCtx, topodatapb.TabletType_REPLICA); err != nil {
+ return vterrors.Wrap(err, "failed to change type to REPLICA")
+ }
+ return nil
  }
 
  // Do a full demotion to convert MySQL into a replica.
  log.Infof("Active reparents are enabled; converting MySQL to replica.")
- if _, err := agent.DemoteMaster(ctx); err != nil {
- return err
+ demoteMasterCtx, cancelDemoteMaster := context.WithTimeout(ctx, *topo.RemoteOperationTimeout)
+ defer cancelDemoteMaster()
+ if _, err := agent.DemoteMaster(demoteMasterCtx); err != nil {
+ return vterrors.Wrap(err, "failed to demote master")
  }
+ setMasterCtx, cancelSetMaster := context.WithTimeout(ctx, *topo.RemoteOperationTimeout)
+ defer cancelSetMaster()
  log.Infof("Attempting to reparent self to new master %v.", masterAliasStr)
- return agent.SetMaster(ctx, masterAlias, 0, true)
+ if err := agent.SetMaster(setMasterCtx, masterAlias, 0, true); err != nil {
+ return vterrors.Wrap(err, "failed to reparent self to new master")
+ }
+ return nil
 }
 
 func (agent *ActionAgent) startShardSync() {

test/vtgate_buffer.py

@@ -37,6 +37,7 @@
 import tablet
 import utils
 from mysql_flavor import mysql_flavor
+from vtproto import topodata_pb2
 
 KEYSPACE = 'ks1'
 SHARD = '0'
@@ -380,6 +381,23 @@ def external_reparent(self):
  utils.run_vtctl(['TabletExternallyReparented', new_master.tablet_alias],
  auto_log=True)
 
+ # Wait until the old master becomes a replica before returning.
+ # Otherwise, that time counts against the timeout of the next test.
+ # Set the timeout well above 30, because it takes 30 seconds just to cancel
+ # stuck transactions when demoting the master.
+ timeout = 60.0
+ while True:
+ try:
+ health = utils.run_vtctl_json(['VtTabletStreamHealth',
+ '-count', '1',
+ old_master.tablet_alias])
+ if health['target']['tablet_type'] == topodata_pb2.REPLICA:
+ break
+ except:
+ pass
+ timeout = utils.wait_step('old master becomes replica', timeout,
+ sleep_time=1.0)
+
 
 class TestBuffer(TestBufferBase):
 
@@ -394,7 +412,7 @@ def test_buffer_planned_reparent(self):
  def planned_reparent():
  utils.run_vtctl(['PlannedReparentShard', '-keyspace_shard',
  '%s/%s' % (KEYSPACE, SHARD),
- '-new_master', replica.tablet_alias])
+ '-new_master', replica.tablet_alias], auto_log=True)
  self._test_buffer(planned_reparent)
 
  def test_buffer_external_reparent(self):