-
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-24150 Allow module tests run in parallel #1464
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,7 +125,7 @@ fi | |
# See http://hbase.apache.org/book.html#maven.release | ||
|
||
echo "Maven details, in case our JDK doesn't match expectations:" | ||
mvn --version --offline | tee "${working_dir}/maven_version" | ||
mvn --threads=2 --version --offline | tee "${working_dir}/maven_version" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no value in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. Dumb change. Undoing. |
||
|
||
echo "Do a clean building of the source artifact using code in ${component_dir}" | ||
cd "${component_dir}" | ||
|
@@ -172,7 +172,7 @@ fi | |
|
||
cd "${unpack_dir}" | ||
echo "Follow the ref guide section on making a RC: Step 8 Build the binary tarball." | ||
if mvn -DskipTests -Prelease --batch-mode -Dmaven.repo.local="${m2_tarbuild}" clean install \ | ||
if mvn --threads=2 -DskipTests -Prelease --batch-mode -Dmaven.repo.local="${m2_tarbuild}" clean install \ | ||
assembly:single >"${working_dir}/srctarball_install.log" 2>&1; then | ||
for artifact in "${unpack_dir}"/hbase-assembly/target/hbase-*-bin.tar.gz; do | ||
if [ -f "${artifact}" ]; then | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not just parallel test suites, but everything. With this change, maven will run up to 2 concurrent modules when the dependency graph permits. I imaging this will be a nice general benefit.
One thing to note though is that some plugins don't support multithreading. IIRC, surefire-reports was one such plugin, and would print warnings in the build. Looks like this patch didn't actually trigger any maven invocation, so I cannot point you at the message. A simple
mvn clean package -DskipTests
should be enough to get the message printed.Since we use Jenkins to aggregate test results, I suggest we disable/remove surefire-reports (this was something I discussed with @busbey sometime back, but neither of us have gotten to it).
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.
Let me change the comment.
I tried running the mvn clean ... above to find the message you refer to but too blind to see it. Let me add change in pom so its generated here.
On disabling the surefire reporting, you want that in this commit or on a follow-on? Is it this you are referring too?
maven-surefire-report-plugin
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.
Yeah that's the plugin. Weirdly, now I try but I don't see the warning message. Hmm..
Running with
-T1.0C
I get a failure inThere 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.
Yeah, I see those WARNINGs locally too. Another 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.
For me it's not just a warning, it breaks the build. Yes another issue, but if it consistently breaks runs using
-T
, it would need to be a blocker of this one.