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

stats: fix and unskip flaky test TestCreateStatsProgress #53681

Merged
merged 1 commit into from
Aug 31, 2020

Conversation

rytaft
Copy link
Collaborator

@rytaft rytaft commented Aug 31, 2020

Release justification: non-production code changes

This commit fixes the flaky test TestCreateStatsProgress and unskips
it. TestCreateStatsProgress was flaky because of the recent changes to
the stats cache, which removed the guarantee that fresh stats would be
available in the cache immediately after stats creation. This commit
fixes the issue by explicitly invalidating the stats cache before the
part of TestCreateStatsProgress that expects certain stats to be present.

Fixes #52782

Release note: None

@rytaft rytaft requested a review from a team as a code owner August 31, 2020 16:28
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @rytaft)


pkg/sql/stats/create_stats_job_test.go, line 610 at r1 (raw file):

	// Invalidate the stats cache so that we can be sure to get the latest stats.
	tableID := descpb.ID(53)

[nit] would be nice to get the table ID programatically (SELECT id FROM system.namespace WHERE name = 't')

Release justification: non-production code changes

This commit fixes the flaky test TestCreateStatsProgress and unskips
it. TestCreateStatsProgress was flaky because of the recent changes to
the stats cache, which removed the guarantee that fresh stats would be
available in the cache immediately after stats creation. This commit
fixes the issue by explicitly invalidating the stats cache before the
part of TestCreateStatsProgress that expects certain stats to be present.

Fixes cockroachdb#52782

Release note: None
Copy link
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


pkg/sql/stats/create_stats_job_test.go, line 610 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] would be nice to get the table ID programatically (SELECT id FROM system.namespace WHERE name = 't')

Done.

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@rytaft
Copy link
Collaborator Author

rytaft commented Aug 31, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 31, 2020

Build succeeded:

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.

opt: TestCreateStatsProgress flaky
3 participants