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

Default chunk size should be 1 instead of 1000 (PT-1423) #813

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

XanC
Copy link

@XanC XanC commented May 8, 2024

There isn't a significant downside to having the chunk-size default to 1, as the chunk resizing algorithm will very quickly scale it up as it can.

There is a big downside to having the chunk-size start higher than 1, in the case of tables whose rows take a long time to chunk.

  • [ N/A ] The contributed code is licensed under GPL v2.0
  • [ x ] Contributor Licence Agreement (CLA) is signed
  • [ N/A ] util/update-modules has been ran
  • [ N/A (automatic) ] Documentation updated
  • [ N/A ] Test suite update

XanC added 2 commits May 6, 2024 14:55
The value of 1000 assumes that the table contains rows which can be checksummed quickly.

Mostly that's true, in which case the auto-sizer will scale chunk-size up very quickly.  But if it isn't true, we need to start at the smallest possible number rather than an arbitrary number.
@XanC XanC requested a review from svetasmirnova as a code owner May 8, 2024 15:46
@it-percona-cla
Copy link

it-percona-cla commented May 8, 2024

CLA assistant check
All committers have signed the CLA.

@svetasmirnova
Copy link
Collaborator

Hi! Thank you for the pull request. For us to accept it, you need to sign the Contributor License Agreement. Please do it.

@XanC
Copy link
Author

XanC commented May 14, 2024

I believe I already did. The comment above yours says "All committers have signed the CLA".

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.

3 participants