Skip to content

Reduce sleep time for finished bulkv2 jobs#5460

Merged
ddanielr merged 3 commits intoapache:2.1from
ddanielr:bulkv2/improve-wait-times
Apr 17, 2025
Merged

Reduce sleep time for finished bulkv2 jobs#5460
ddanielr merged 3 commits intoapache:2.1from
ddanielr:bulkv2/improve-wait-times

Conversation

@ddanielr
Copy link
Contributor

@ddanielr ddanielr commented Apr 9, 2025

Skips the sleep calculation when bulkv2 returns 1 or 0 as success cases.
Also removes an unnecessary max comparison as sleepTime will always be less than or equal to locationLess

Skips the sleep calculation when bulkv2 reports that it has waited for
all tablet servers to respond.
@ddanielr ddanielr added this to the 2.1.4 milestone Apr 9, 2025
@ctubbsii
Copy link
Member

I'm having a hard time understanding how these changes match the description:
Skips the sleep calculation when bulkv2 reports that it has waited for all tablet servers to respond. or Reduce sleep time for finished bulkv2 jobs

It seems like the only substantive change was the change from > 0 to (effectively) > 1, so it only skips computing a new sleepTime when it is exactly 1 (it was already skipped when it was 0). Does the value of 1 have special meaning? If so, I think there's a simpler to change to just change to > 1 and add a small comment explaining why we're skipping when it's that particular value.

…Ops/bulkVer2/LoadFiles.java


Simplify sleep logic

Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>
@ctubbsii
Copy link
Member

@ddanielr, @keith-turner suggested an improvement to the log message. I see you marked that comment thread as resolved, but did not implement the suggested changes. I don't care one way or the other, but wanted to bring it to your attention before this is merged in case you overlooked it.

@ddanielr
Copy link
Contributor Author

@ddanielr, @keith-turner suggested an improvement to the log message. I see you marked that comment thread as resolved, but did not implement the suggested changes. I don't care one way or the other, but wanted to bring it to your attention before this is merged in case you overlooked it.

Yep I see that and have updated the comment message.

@ddanielr ddanielr merged commit 794861a into apache:2.1 Apr 17, 2025
8 checks passed
@ddanielr ddanielr deleted the bulkv2/improve-wait-times branch April 17, 2025 13:03
ddanielr added a commit to keith-turner/accumulo that referenced this pull request Apr 24, 2025
* Skips the sleep calculation when bulkv2 returns 1 or 0 as success cases.

Also removes an unnecessary max comparison as sleepTime will always be less than or equal to locationLess
* Reduce sleep time for finished bulkv2 jobs

---------

Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>
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