From 18d8322cf206e54a6cf7ee380ca012ae0e1e2a68 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 21 May 2024 20:47:17 +0530 Subject: [PATCH] feat: address review comments Signed-off-by: Manan Gupta --- go/test/endtoend/reparent/newfeaturetest/reparent_test.go | 2 +- go/vt/vtctl/reparentutil/emergency_reparenter.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/go/test/endtoend/reparent/newfeaturetest/reparent_test.go b/go/test/endtoend/reparent/newfeaturetest/reparent_test.go index b56d3195b32..ad798d61792 100644 --- a/go/test/endtoend/reparent/newfeaturetest/reparent_test.go +++ b/go/test/endtoend/reparent/newfeaturetest/reparent_test.go @@ -175,5 +175,5 @@ func TestERSWithWriteInPromoteReplica(t *testing.T) { "set sql_log_bin=1", }, tablets[3]) _, err := utils.Ers(clusterInstance, tablets[3], "60s", "30s") - require.NoError(t, err, "ERS should not fail even if there is a sidecard db change") + require.NoError(t, err, "ERS should not fail even if there is a sidecardb change") } diff --git a/go/vt/vtctl/reparentutil/emergency_reparenter.go b/go/vt/vtctl/reparentutil/emergency_reparenter.go index afc750dc667..a8d71186b30 100644 --- a/go/vt/vtctl/reparentutil/emergency_reparenter.go +++ b/go/vt/vtctl/reparentutil/emergency_reparenter.go @@ -487,7 +487,10 @@ func (erp *EmergencyReparenter) reparentReplicas( tabletMap map[string]*topo.TabletInfo, statusMap map[string]*replicationdatapb.StopReplicationStatus, opts EmergencyReparentOptions, - intermediateReparent bool, + intermediateReparent bool, // intermediateReparent represents whether the reparenting of the replicas is the final reparent or not. + // Since ERS can sometimes promote a tablet, which isn't a candidate for promotion, if it is the most advanced, we don't want to + // call PromoteReplica on it. We just want to get all replicas to replicate from it to get caught up, after which we'll promote the primary + // candidate separately. During the final promotion, we call `PromoteReplica` and `PopulateReparentJournal`. ) ([]*topodatapb.Tablet, error) { var ( @@ -611,7 +614,7 @@ func (erp *EmergencyReparenter) reparentReplicas( erp.logger.Errorf("failed to promote %s to primary", topoproto.TabletAliasString(newPrimaryTablet.Alias)) replCancel() - return nil, vterrors.Wrapf(primaryErr, "failed to promote %v to primary: %v", topoproto.TabletAliasString(newPrimaryTablet.Alias), primaryErr) + return nil, vterrors.Wrapf(primaryErr, "failed to promote %v to primary", topoproto.TabletAliasString(newPrimaryTablet.Alias)) } // We should only cancel the context that all the replicas are using when they are done.