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

tm: add more logging to checkMastership #6618

Merged
merged 2 commits into from
Aug 26, 2020
Merged

tm: add more logging to checkMastership #6618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e6f463
tm: add more logging to checkMastership
sougou Aug 25, 2020
5fc7992
tm: checkMastership: handle more corner cases
sougou Aug 25, 2020
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
11 changes: 11 additions & 0 deletions go/vt/vttablet/tabletmanager/tm_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ func (tm *TabletManager) checkMastership(ctx context.Context, si *topo.ShardInfo
case topo.IsErrType(err, topo.NoNode):
// There's no existing tablet record, so we can assume
// no one has left us a message to step down.
log.Infof("Shard master alias matches, but there is no existing tablet record. Switching to master with 'Now' as time")
tm.tmState.UpdateTablet(func(tablet *topodatapb.Tablet) {
tablet.Type = topodatapb.TabletType_MASTER
// Update the master term start time (current value is 0) because we
Expand All @@ -468,12 +469,19 @@ func (tm *TabletManager) checkMastership(ctx context.Context, si *topo.ShardInfo
})
case err == nil:
if oldTablet.Type == topodatapb.TabletType_MASTER {
log.Infof("Shard master alias matches, and existing tablet agrees. Switching to master with tablet's master term start time: %v", oldTablet.MasterTermStartTime)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should also log if oldTablet.Type was not MASTER? That means the shard record and tablet record disagree, which seems like an important case to call out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. In this case, would it be safe to make ourselves master and match the shard record's timestamp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added handling for a couple of more corner cases. LMK what you think.

// We're marked as master in the shard record,
// and our existing tablet record agrees.
tm.tmState.UpdateTablet(func(tablet *topodatapb.Tablet) {
tablet.Type = topodatapb.TabletType_MASTER
tablet.MasterTermStartTime = oldTablet.MasterTermStartTime
})
} else {
log.Warningf("Shard master alias matches, but existing tablet is not master. Switching to master with the shard's master term start time: %v", oldTablet.MasterTermStartTime)
tm.tmState.UpdateTablet(func(tablet *topodatapb.Tablet) {
deepthi marked this conversation as resolved.
Show resolved Hide resolved
tablet.Type = topodatapb.TabletType_MASTER
tablet.MasterTermStartTime = si.MasterTermStartTime
})
}
default:
return vterrors.Wrap(err, "InitTablet failed to read existing tablet record")
Expand All @@ -490,10 +498,13 @@ func (tm *TabletManager) checkMastership(ctx context.Context, si *topo.ShardInfo
oldMasterTermStartTime := oldTablet.GetMasterTermStartTime()
currentShardTime := si.GetMasterTermStartTime()
if oldMasterTermStartTime.After(currentShardTime) {
log.Infof("Shard master alias does not match, but the tablet's master term start time is newer. Switching to master with tablet's master term start time: %v", oldTablet.MasterTermStartTime)
tm.tmState.UpdateTablet(func(tablet *topodatapb.Tablet) {
tablet.Type = topodatapb.TabletType_MASTER
tablet.MasterTermStartTime = oldTablet.MasterTermStartTime
})
} else {
log.Infof("Existing tablet type is master, but the shard record has a different master with a newer timestamp. Remaining a replica")
}
}
default:
Expand Down
29 changes: 26 additions & 3 deletions go/vt/vttablet/tabletmanager/tm_init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,23 @@ func TestCheckMastership(t *testing.T) {
// 2. Update shard's master to our alias, then try to init again.
// (This simulates the case where the MasterAlias in the shard record says
// that we are the master but the tablet record says otherwise. In that case,
// we assume we are not the MASTER.)
// we become master by inheriting the shard record's timestamp.)
now := time.Now()
_, err = ts.UpdateShardFields(ctx, "ks", "0", func(si *topo.ShardInfo) error {
si.MasterAlias = alias
si.MasterTermStartTime = logutil.TimeToProto(now)
// Reassign to now for easier comparison.
now = si.GetMasterTermStartTime()
return nil
})
require.NoError(t, err)
err = tm.Start(tablet, 0)
require.NoError(t, err)
ti, err = ts.GetTablet(ctx, alias)
require.NoError(t, err)
assert.Equal(t, topodatapb.TabletType_REPLICA, ti.Type)
assert.Equal(t, topodatapb.TabletType_MASTER, ti.Type)
ter0 := ti.GetMasterTermStartTime()
assert.True(t, ter0.IsZero())
assert.Equal(t, now, ter0)
tm.Stop()

// 3. Delete the tablet record. The shard record still says that we are the
Expand Down Expand Up @@ -291,6 +295,25 @@ func TestCheckMastership(t *testing.T) {
ter4 := ti.GetMasterTermStartTime()
assert.Equal(t, ter1, ter4)
tm.Stop()

// 7. If the shard record shows a different master with a newer
// timestamp, we remain replica.
_, err = ts.UpdateShardFields(ctx, "ks", "0", func(si *topo.ShardInfo) error {
si.MasterAlias = otherAlias
si.MasterTermStartTime = logutil.TimeToProto(ter4.Add(10 * time.Second))
return nil
})
require.NoError(t, err)
tablet.Type = topodatapb.TabletType_REPLICA
tablet.MasterTermStartTime = nil
err = tm.Start(tablet, 0)
require.NoError(t, err)
ti, err = ts.GetTablet(ctx, alias)
require.NoError(t, err)
assert.Equal(t, topodatapb.TabletType_REPLICA, ti.Type)
ter5 := ti.GetMasterTermStartTime()
assert.True(t, ter5.IsZero())
tm.Stop()
}

func TestStartCheckMysql(t *testing.T) {
Expand Down