Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error message for planned reparent shard #15529

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Fix error message for planned reparent shard #15529

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4564829
feat: fix error message for planned reparent shard
GuptaManan100 Mar 20, 2024
6693c85
feat: improve performance
GuptaManan100 Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion go/vt/vtctl/reparentutil/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,23 @@ func ElectNewPrimary(

// candidates are the list of tablets that can be potentially promoted after filtering out based on preliminary checks.
candidates := []*topodatapb.Tablet{}
var reasonsToInvalidate string
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
for _, tablet := range tabletMap {
switch {
case newPrimaryAlias != nil:
// If newPrimaryAlias is provided, then that is the only valid tablet, even if it is not of type replica or in a different cell.
if !topoproto.TabletAliasEqual(tablet.Alias, newPrimaryAlias) {
reasonsToInvalidate += fmt.Sprintf("\n%v does not match the new primary alias provided", topoproto.TabletAliasString(tablet.Alias))
continue
}
case primaryCell != "" && tablet.Alias.Cell != primaryCell:
reasonsToInvalidate += fmt.Sprintf("\n%v is not in the same cell as the previous primary", topoproto.TabletAliasString(tablet.Alias))
continue
case avoidPrimaryAlias != nil && topoproto.TabletAliasEqual(tablet.Alias, avoidPrimaryAlias):
reasonsToInvalidate += fmt.Sprintf("\n%v matches the primary alias to avoid", topoproto.TabletAliasString(tablet.Alias))
continue
case tablet.Tablet.Type != topodatapb.TabletType_REPLICA:
reasonsToInvalidate += fmt.Sprintf("\n%v is not a replica", topoproto.TabletAliasString(tablet.Alias))
continue
}

Expand All @@ -124,6 +129,8 @@ func ElectNewPrimary(
if err == nil && (tolerableReplLag == 0 || tolerableReplLag >= replLag) {
validTablets = append(validTablets, tb)
tabletPositions = append(tabletPositions, pos)
} else {
reasonsToInvalidate += fmt.Sprintf("\n%v has %v replication lag which is more than the tolerable amount", topoproto.TabletAliasString(tablet.Alias), replLag)
}
return err
})
Expand All @@ -136,7 +143,7 @@ func ElectNewPrimary(

// return an error if there are no valid tablets available
if len(validTablets) == 0 {
return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "cannot find a tablet to reparent to in the same cell as the current primary")
return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "cannot find a tablet to reparent to%v", reasonsToInvalidate)
}

// sort the tablets for finding the best primary
Expand Down
58 changes: 38 additions & 20 deletions go/vt/vtctl/reparentutil/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestElectNewPrimary(t *testing.T) {
avoidPrimaryAlias *topodatapb.TabletAlias
tolerableReplLag time.Duration
expected *topodatapb.TabletAlias
shouldErr bool
errContains []string
}{
{
name: "found a replica",
Expand Down Expand Up @@ -135,7 +135,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 102,
},
shouldErr: false,
errContains: nil,
},
{
name: "new primary alias provided - no tolerable replication lag",
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
shouldErr: false,
errContains: nil,
},
{
name: "new primary alias provided - with tolerable replication lag",
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 102,
},
shouldErr: false,
errContains: nil,
},
{
name: "new primary alias provided - with intolerable replication lag",
Expand Down Expand Up @@ -284,8 +284,13 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 102,
},
expected: nil,
shouldErr: true,
expected: nil,
errContains: []string{
`cannot find a tablet to reparent to`,
`zone1-0000000100 does not match the new primary alias provided`,
`zone1-0000000101 does not match the new primary alias provided`,
`zone1-0000000102 has 1m40s replication lag which is more than the tolerable amount`,
},
},
{
name: "found a replica ignoring replica lag",
Expand Down Expand Up @@ -345,7 +350,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 102,
},
shouldErr: false,
errContains: nil,
},
{
name: "found a replica - ignore one with replication lag",
Expand Down Expand Up @@ -405,7 +410,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
shouldErr: false,
errContains: nil,
},
{
name: "found a replica - more advanced relay log position",
Expand Down Expand Up @@ -465,7 +470,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 102,
},
shouldErr: false,
errContains: nil,
},
{
name: "no active primary in shard",
Expand Down Expand Up @@ -505,7 +510,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
shouldErr: false,
errContains: nil,
},
{
name: "avoid primary alias is nil",
Expand Down Expand Up @@ -547,7 +552,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
shouldErr: false,
errContains: nil,
}, {
name: "avoid primary alias and shard primary are nil",
tmc: &chooseNewPrimaryTestTMClient{
Expand Down Expand Up @@ -586,7 +591,7 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
shouldErr: false,
errContains: nil,
},
{
name: "no replicas in primary cell",
Expand Down Expand Up @@ -640,8 +645,13 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 0,
},
expected: nil,
shouldErr: true,
expected: nil,
errContains: []string{
`cannot find a tablet to reparent to`,
`zone2-0000000200 is not a replica`,
`zone1-0000000101 is not in the same cell as the previous primary`,
`zone1-0000000102 is not in the same cell as the previous primary`,
},
},
{
name: "only available tablet is AvoidPrimary",
Expand Down Expand Up @@ -677,8 +687,11 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 101,
},
expected: nil,
shouldErr: true,
expected: nil,
errContains: []string{
`cannot find a tablet to reparent to
zone1-0000000101 matches the primary alias to avoid`,
},
},
{
name: "no replicas in shard",
Expand All @@ -704,8 +717,11 @@ func TestElectNewPrimary(t *testing.T) {
Cell: "zone1",
Uid: 0,
},
expected: nil,
shouldErr: true,
expected: nil,
errContains: []string{
`cannot find a tablet to reparent to
zone1-0000000100 is not a replica`,
},
},
}

Expand All @@ -716,8 +732,10 @@ func TestElectNewPrimary(t *testing.T) {
t.Parallel()

actual, err := ElectNewPrimary(ctx, tt.tmc, tt.shardInfo, tt.tabletMap, tt.newPrimaryAlias, tt.avoidPrimaryAlias, time.Millisecond*50, tt.tolerableReplLag, durability, logger)
if tt.shouldErr {
assert.Error(t, err)
if len(tt.errContains) > 0 {
for _, errC := range tt.errContains {
assert.ErrorContains(t, err, errC)
}
return
}

Expand Down
Loading