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

logictest: ensure lower bound on bytes limit for sqlite #92631

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Nov 28, 2022

This commit makes it so that defaultBatchBytesLimit is set to at least 3KiB when running sqlite logic tests since if that value is too low, the tests can take really long time (in one example with value of 163 bytes it took 25 minutes vs 3 minutes with 2.5KiB value). This is achieved by explicitly updating this single metamorphic value when run with the sqlite target. I chose this option rather than forcing production values knob (which would disable some other randomizations) to have the smallest possible reduction in test coverage while still making the tests fast enough.

Fixes: #92534.

Release note: None

@yuzefovich yuzefovich requested review from rytaft, DrewKimball and a team November 28, 2022 23:58
@yuzefovich yuzefovich requested a review from a team as a code owner November 28, 2022 23:58
Copy link
Collaborator

@DrewKimball DrewKimball left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -3999,6 +4000,9 @@ type TestServerArgs struct {
// If set, then we will disable the metamorphic randomization of
// smallEngineBlocks variable.
DisableSmallEngineBlocks bool
// If set, then default-batch-bytes-limit metamorphic constant is guaranteed
// to be at least 1KiB.
EnableLowerBoundOnBatchBytesLimit bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] could you make this the lower bound in bytes instead, and just set it to 1KiB here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I also decided to bump it up to 3KiB min.

This commit makes it so that `defaultBatchBytesLimit` is set to at least
3KiB when running sqlite logic tests since if that value is too low, the
tests can take really long time (in one example with value of 163 bytes
it took 25 minutes vs 3 minutes with 2.5KiB value). This is achieved by
explicitly updating this single metamorphic value when run with the
sqlite target. I chose this option rather than forcing production values
knob (which would disable some other randomizations) to have the
smallest possible reduction in test coverage while still making the
tests fast enough.

Release note: None
@yuzefovich
Copy link
Member Author

TFTR!

bors r+

@DrewKimball
Copy link
Collaborator

LGTM

@craig
Copy link
Contributor

craig bot commented Nov 29, 2022

Build failed (retrying...):

craig bot pushed a commit that referenced this pull request Nov 29, 2022
92463: server, ui: add used index to statement details r=maryliag a=maryliag

This commit convert from table id and index id on
the statement details endpoint, and adds the index used information to the Explain Plan tab on Statement Details.

The elements of the list are links that redirect to the table or index details page.

Fixes #82615

https://www.loom.com/share/530bf4e795d648bb854c18d60b074e4c

<img width="1483" alt="Screen Shot 2022-11-24 at 11 11 54 AM" src="https://user-images.githubusercontent.com/1017486/203828306-0a6fb905-cae1-4217-8ea3-b4e323cf3a72.png">



Release note (ui change): Add a list of used index per explain plan, under the Explain Plan tab on Statement Details page, with links to the table or index details pages.

92631: logictest: ensure lower bound on bytes limit for sqlite r=yuzefovich a=yuzefovich

This commit makes it so that `defaultBatchBytesLimit` is set to at least 3KiB when running sqlite logic tests since if that value is too low, the tests can take really long time (in one example with value of 163 bytes it took 25 minutes vs 3 minutes with 2.5KiB value). This is achieved by explicitly updating this single metamorphic value when run with the sqlite target. I chose this option rather than forcing production values knob (which would disable some other randomizations) to have the smallest possible reduction in test coverage while still making the tests fast enough.

Fixes: #92534.

Release note: None

Co-authored-by: maryliag <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig
Copy link
Contributor

craig bot commented Nov 29, 2022

This PR was included in a batch that timed out, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Nov 29, 2022

Build failed:

@yuzefovich
Copy link
Member Author

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 29, 2022

Build succeeded:

@craig craig bot merged commit edc6fda into cockroachdb:master Nov 29, 2022
@yuzefovich yuzefovich deleted the sqlite branch November 29, 2022 23:28
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.

sqlitelogictest: 2-hour timeout may not be enough
2 participants