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

Changes from 1 commit
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
3 changes: 3 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,6 +469,7 @@ 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) {
Expand All @@ -490,6 +492,7 @@ 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
Expand Down