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.

Sign up for GitHub

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

Jump to bottom

Improve BulkSplitOptimizationIT #5179

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve BulkSplitOptimizationIT #5179

wants to merge 2 commits into from

Conversation

DomGarguilo
Copy link
Member

Partially addresses #5123

The most important change in this PR is just to bump the timeout from 2 to 5 minutes since the test often times out

I also...

  • added logging to make it easier to tell how far the test got and where it times out at in the future
  • moved the creation of the rfiles to the before block
  • created the test table using NewTableConfiguration and made the initial availability HOSTED to speed things up slightly

@DomGarguilo DomGarguilo added the test An issue pertaining to a test case label Dec 13, 2024
@DomGarguilo DomGarguilo added this to the 4.0.0 milestone Dec 13, 2024
@DomGarguilo DomGarguilo self-assigned this Dec 13, 2024
keith-turner
keith-turner approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@keith-turner keith-turner left a comment

Choose a reason for hiding this comment

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

With the new logging you added do you know where the most time is being spent in the test?

@DomGarguilo
Copy link
Member Author

With the new logging you added do you know where the most time is being spent in the test?

I added some timing logging to the test to find out. Here is what the logs show:

Took 31778 ms to stop all tservers
Took 3 ms to start all tservers
Generated 100 files in 403 ms
Created table in 23334 ms
Imported into table in 601 ms
Took 3031 ms for split count to reach expected range

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keith-turner keith-turner keith-turner approved these changes

Assignees

@DomGarguilo DomGarguilo

Labels
test An issue pertaining to a test case
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@DomGarguilo @keith-turner