-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-23779 Up the default fork count to make builds complete faster;… #1461
Conversation
(!) A patch to the testing environment has been detected. |
2 similar comments
(!) A patch to the testing environment has been detected. |
(!) A patch to the testing environment has been detected. |
🎊 +1 overall
This message was automatically generated. |
dev-support/hbase-personality.sh
Outdated
# See below for more on -T: | ||
# https://cwiki.apache.org/confluence/display/MAVEN/Parallel+builds+in+Maven+3 | ||
forkcount="0.5C" | ||
extra="-T${forkcount} -Dsurefire.firstPartForkCount=${forkcount} -Dsurefire.secondPartForkCount=${forkcount} -DHBasePatchProcess" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does maven --threads
interact with surefire forks? Isn't it the case that we get forkCount
maven process thread each building a module simultaneously, and each of those modules is launching up to forkCount
concurrent test jvms? Means we're looking at 64 concurrent test processes? Seems high to me.
If it is indeed launching like this, I would want -T{forkCount}
when running compile/install
, but i want just a single core when running test
.
I took a read through the surefire docs but I don't have a better handle on this than when I started.
If Maven is instead managing an upper-bound on concurrent test jvms at ${forkcount}
, I say +1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review.
bq. How does maven --threads interact with surefire forks?
There is no correlation. There may be cases where it acts as a multiplier where modules are independent of each other but generally, there is little opportunity for -T benefit in hbase builds.
I went back and checked a few machines. Couldn't find instance where fork count rose above forkcount limit. Doesn't mean it doesn't happen.
Let me untangle the two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified the patch after the above. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... on big box, with 1.0C and no T, build takes 1hr 15mins. With 0.5C and T==2, takes 53minutes. There are probably spurts of lots of processes but interesting that overall time taken goes down significantly. Let me come back and set the T in another follow-on issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened HBASE-24150 Might be more speed up there than here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one, I left you a comment over there too.
I wonder if there's a docker param that limits the number of processes a container can launch... could set that to the expected max parallel jvms and see if it breaches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will poke around....
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
(!) A patch to the testing environment has been detected. |
2 similar comments
(!) A patch to the testing environment has been detected. |
(!) A patch to the testing environment has been detected. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
(!) A patch to the testing environment has been detected. |
2 similar comments
(!) A patch to the testing environment has been detected. |
(!) A patch to the testing environment has been detected. |
🎊 +1 overall
This message was automatically generated. |
Rerunning tests. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
dev-support/hbase-personality.sh
Outdated
@@ -140,7 +140,7 @@ function personality_modules | |||
|
|||
clear_personality_queue | |||
|
|||
extra="-DHBasePatchProcess" | |||
extra=" -DHBasePatchProcess" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ws.
… make count relative to CPU count Halve the build speed by doubling the -C count from 0.25C to 0.5C.
Fix the white space. Re-run. I don't like the test failures I'm seeing. jenkins doesn't like this 0.5C for forkcount. HBASE-24150 might have higher yield and be more stable. Lets see. Will see how this build goes and then will push a non-change for compare. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Closing for now. Too destabilizing. Closing in favor of the approach over in HBASE-24150. Upping the forkcount to 0.5C threatens to destabilize builds, especially after HBASE-24150 (currently merged to branch-2 for survey) |
Closing for now. Too destabilizing. Closing in favor of the approach over in HBASE-24150. Upping the forkcount to 0.5C threatens to destabilize builds, especially after HBASE-24150 (currently merged to branch-2 for survey) |
… make count relative to CPU count
Halve the build speed by doubling the -C count from 0.25C to 0.5C.
Pass mvn a -T of 0.5C too.