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.

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

Correctly initialize the TabletType stats #6989

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

dweitzman
Copy link
Member

Fixes #6988

Signed-off-by: David Weitzman [email protected]

@dweitzman dweitzman marked this pull request as draft November 3, 2020 19:00
@dweitzman
Copy link
Member Author

Converted to a draft because I noticed in testing that a tablet can still have the wrong TabletType in stats. I had a tablet started with type "spare" are converted to master that still describes itself as "spare".

@dweitzman dweitzman force-pushed the fix_statsTabletType branch from d04f58f to 78481aa Compare November 3, 2020 19:19
@dweitzman
Copy link
Member Author

Fixed to set the initial stat after checkMastership runs

@dweitzman dweitzman marked this pull request as ready for review November 3, 2020 19:20
@@ -283,6 +283,7 @@ func (tm *TabletManager) Start(tablet *topodatapb.Tablet, healthCheckInterval ti

// The following initializations don't need to be done
// in any specific order.
statsTabletType.Set(topoproto.TabletTypeLString(tm.tmState.tablet.Type))
Copy link
Member

Choose a reason for hiding this comment

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

Won't it be better to do this inside exportStats? Similar to how we are setting keyspace and shard stats.

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes a lot of sense to me. Updated

@dweitzman dweitzman force-pushed the fix_statsTabletType branch from 78481aa to 060bc8e Compare November 5, 2020 19:42
@sougou sougou merged commit d2a42d5 into vitessio:master Nov 6, 2020
@askdba askdba added this to the v9.0 milestone Nov 11, 2020
@deepthi
Copy link
Member

deepthi commented Jan 27, 2021

We should backport this to 7.0 and 8.0. Also statsTabletTypeCount needs to be fixed similarly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: statsTabletType not initialized in 7.0.3
4 participants