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

healthcheck: Cache regions in tablet_stats_cache #4619

Merged
merged 3 commits into from
Feb 15, 2019

Conversation

leoxlin
Copy link

@leoxlin leoxlin commented Feb 13, 2019

This adds caches getRegionByCell call to improve initial vtgate healthcheck performance.

We've seen some significant initial healthcheck slow down in our recent vitess upgrade that resulted in the gateway timing out before finishing waiting for the -tablet_types_to_wait

We were able to diagnose this via a lock contention profile that showed StatsUpdate to be the pain point. From there @sougou was able to identify the uncached cell region call.

Signed-off-by: Leo Xuzhang Lin [email protected]

@leoxlin
Copy link
Author

leoxlin commented Feb 13, 2019


func (tc *TabletStatsCache) getOrCreateRegionByCell(cell string) string {
// Fast path
if region, ok := tc.getRegionByCell(cell); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for a separate fast path. You can just check of tc.cellRegions inside the 'slow' path. It's essentially the same thing. Which means you don't need a separate getRegionByCell function. Which means you could just rename this function to getRegionByCell.

@sougou
Copy link
Contributor

sougou commented Feb 13, 2019

Also, there's a new test failure.

Leo Xuzhang Lin added 2 commits February 13, 2019 16:44
Signed-off-by: Leo Xuzhang Lin <[email protected]>
Signed-off-by: Leo Xuzhang Lin <[email protected]>
@leoxlin
Copy link
Author

leoxlin commented Feb 14, 2019

I've addressed all comments and fixed the test

@sougou sougou merged commit dd8cb89 into vitessio:master Feb 15, 2019
@leoxlin leoxlin deleted the cache_cell_regions branch February 19, 2019 11:28
systay pushed a commit that referenced this pull request Jul 22, 2024
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.

2 participants