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

coldata: fix recent flaky behavior #94661

Merged
merged 1 commit into from
Jan 3, 2023
Merged

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Jan 3, 2023

In a just merged b353d18 when addressing the PR feedback I introduced a possibility of a crash in tests. In particular, this can occur if we choose to randomize coldata.BatchSize value larger than the default (or if the test explicitly overrides it). The problem is that BatchSize() can return different values in init() of coldata package and during the test runs, so we now use MaxBatchSize where applicable.

Epic: None

Release note: None

@yuzefovich yuzefovich requested review from michae2, DrewKimball and a team January 3, 2023 21:48
@cockroach-teamcity
Copy link
Member

This change is Reviewable

In a just merged b353d18 when
addressing the PR feedback I introduced a possibility of a crash in
tests. In particular, this can occur if we choose to randomize
`coldata.BatchSize` value larger than the default (or if the test
explicitly overrides it). The problem is that `BatchSize()` can return
different values in `init()` of `coldata` package and during the test
runs, so we now use `MaxBatchSize` where applicable.

Release note: None
Copy link
Collaborator

@rharding6373 rharding6373 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 @DrewKimball and @michae2)

@yuzefovich
Copy link
Member Author

TFTR!

bors r+

Copy link
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

:lgtm:

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

@craig
Copy link
Contributor

craig bot commented Jan 3, 2023

Build succeeded:

@craig craig bot merged commit 5c892eb into cockroachdb:master Jan 3, 2023
@yuzefovich yuzefovich deleted the fix-bytes branch January 3, 2023 23:10
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.

4 participants