Skip to content

sql,backfill: deflake TestValidationWithProtectedTS#109010

Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom
rafiss:deflake-TestValidationWithProtectedTS
Aug 18, 2023
Merged

sql,backfill: deflake TestValidationWithProtectedTS#109010
craig[bot] merged 3 commits intocockroachdb:masterfrom
rafiss:deflake-TestValidationWithProtectedTS

Conversation

@rafiss
Copy link
Copy Markdown
Collaborator

@rafiss rafiss commented Aug 18, 2023

This test was slow enough to the point of flaking. I tracked down the
problem to the metamorphic constant kv-batch-size. Now we set a
testing knob to make sure that the value is always the production value.

This also required a fix to plumb the testing knob through in one more
place.

fixes #106960
informs #108539
Release note: None

rafiss added 2 commits August 18, 2023 12:01
This test was slow enough to the point of flaking. I tracked down the
problem to the metamorphic constant `kv-batch-size`. Now we set a
testing knob to make sure that the value is always the production value.

This also required a fix to plumb the testing knob through in one more
place.

Release note: None
…IC_TESTING

The interpretation of this flag was flipped.

Release note: None
@rafiss rafiss requested review from fqazi and yuzefovich August 18, 2023 16:03
@rafiss rafiss requested a review from a team as a code owner August 18, 2023 16:03
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

@rafiss Thanks for doing this! :lgtm:

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)

This test was timing out sometimes due to low batch sizes.

Release note: None
@rafiss rafiss force-pushed the deflake-TestValidationWithProtectedTS branch from c5f7805 to 7a701ec Compare August 18, 2023 16:44
@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Aug 18, 2023

tftr!

bors r+

Copy link
Copy Markdown
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

Nice work tracking this down! :lgtm:

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @rafiss)


pkg/sql/backfill/backfill.go line 173 at r1 (raw file):

			MemMonitor: cb.mon,
			Spec:       &spec,
			TraceKV:    traceKV,

Perhaps we should set the knob here for consistency too?

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 18, 2023

Build succeeded:

@craig craig bot merged commit cf87437 into cockroachdb:master Aug 18, 2023
@rafiss rafiss deleted the deflake-TestValidationWithProtectedTS branch August 21, 2023 20:58
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.

sql: speed up TestValidationWithProtectedTS under multi-tenancy

4 participants