Skip to content

HBASE-29838 Run Hadoop Check as a GitHub Action#7651

Open
ndimiduk wants to merge 2 commits intoapache:masterfrom
ndimiduk:29838-jdk-checks-master
Open

HBASE-29838 Run Hadoop Check as a GitHub Action#7651
ndimiduk wants to merge 2 commits intoapache:masterfrom
ndimiduk:29838-jdk-checks-master

Conversation

@ndimiduk
Copy link
Member

No description provided.

@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@ndimiduk
Copy link
Member Author

Looks like we occupied the runner for 6h and then it was aborted.

@Apache9
Copy link
Contributor

Apache9 commented Jan 20, 2026

https://infra.apache.org/github-actions-policy.html

The policy here does not say about the 6 hours timeout...

We can ask infra about the rules and the size of the github runners, our jenkins runners finished in "343m 23s", which was very close to 6 hours, so if the machine of the github runner is weaker, the build will be very easy to cost more than 6 hours...

@ndimiduk
Copy link
Member Author

My action itself has timeout-minutes: 600 . i'll try bumping this up.

@ndimiduk
Copy link
Member Author

or not. 6h is GH's hard limit, https://docs.github.com/en/actions/reference/limits

@Apache9
Copy link
Contributor

Apache9 commented Jan 21, 2026

Then maybe we should try self hosted github runners? For self hosted runners the execution time limit is 5 days...

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from 1829212 to 352a9a1 Compare January 23, 2026 07:03
@ndimiduk
Copy link
Member Author

Then maybe we should try self hosted github runners? For self hosted runners the execution time limit is 5 days...

Yes we should bring this back to our CI discussions with Infra. Maybe we can borrow from the pool of new Jenkins workers while we continue to build this out.

Yetus is supposed to provide smart, selective detection of module changes when it decided which tests to run. I think the new .github directory broke that for this run, so I've pushed a change to exclude it, maybe that will help.

I'm also going to see if I can manually parallelize the unit test runs -- maybe break out three separate checks for the three main unit test groups or something like that.

@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache9
Copy link
Contributor

Apache9 commented Jan 23, 2026

Then maybe we should try self hosted github runners? For self hosted runners the execution time limit is 5 days...

Yes we should bring this back to our CI discussions with Infra. Maybe we can borrow from the pool of new Jenkins workers while we continue to build this out.

Yetus is supposed to provide smart, selective detection of module changes when it decided which tests to run. I think the new .github directory broke that for this run, so I've pushed a change to exclude it, maybe that will help.

I'm also going to see if I can manually parallelize the unit test runs -- maybe break out three separate checks for the three main unit test groups or something like that.

Maybe we could split the UTs run as a seperated github check? Or even more, we could split the UTs run as two seperated check, one has -PrunDevTests(for small/medium tests) and one has -PrunLargeTests(for large tests).

@ndimiduk
Copy link
Member Author

Maybe we could split the UTs run as a seperated github check? Or even more, we could split the UTs run as two seperated check, one has -PrunDevTests(for small/medium tests) and one has -PrunLargeTests(for large tests).

Yep, that's exactly my thinking as well. Landing these other cleanup issues and I'll be back.

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from 352a9a1 to adfd056 Compare January 23, 2026 12:48
@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@ndimiduk
Copy link
Member Author

Okay this is better. Module selection chose only hbase-examples for running the unit tests.

-1 overall
  
  | Vote |      Subsystem |  Runtime   | Comment
  ============================================================================
  |   0  |        reexec  |   1m 13s   | Docker mode activated. 
  |  -0  |         yetus  |   0m  4s   | Unprocessed flag(s): --brief-report-file 
  |      |                |            | --spotbugs-strict-precheck
  |      |                |            | --author-ignore-list
  |      |                |            | --blanks-eol-ignore-file
  |      |                |            | --blanks-tabs-ignore-file
  |      |                |            | --quick-hadoopcheck
  +---------------------------------------------------------------------------
  |      |                |            | Prechecks 
  +---------------------------------------------------------------------------
  +---------------------------------------------------------------------------
  |      |                |            | master Compile Tests 
  +---------------------------------------------------------------------------
  |  +1  |    mvninstall  |   3m 25s   | master passed 
  |  +1  |       compile  |   0m 19s   | master passed 
  |  +1  |       javadoc  |   0m 17s   | master passed 
  |  +1  |    shadedjars  |   4m 47s   | branch has no errors when building our 
  |      |                |            | shaded downstream artifacts.
  +---------------------------------------------------------------------------
  |      |                |            | Patch Compile Tests 
  +---------------------------------------------------------------------------
  |  +1  |    mvninstall  |   3m  7s   | the patch passed 
  |  +1  |       compile  |   0m 18s   | the patch passed 
  |  +1  |         javac  |   0m 18s   | the patch passed 
  |  +1  |       javadoc  |   0m 15s   | the patch passed 
  |  +1  |    shadedjars  |   4m 51s   | patch has no errors when building our 
  |      |                |            | shaded downstream artifacts.
  +---------------------------------------------------------------------------
  |      |                |            | Other Tests 
  +---------------------------------------------------------------------------
  |  -1  |          unit  |   0m 24s   | hbase-examples in the patch failed. 
  |      |                |  19m 12s   |
  
  
  || Subsystem || Report/Notes ||
  ============================================================================
  |  unit  | /home/runner/work/hbase/hbase/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-examples.txt |
  | Docker | ClientAPI=1.48 ServerAPI=1.48 base: /home/runner/work/hbase/hbase/yetus-jdk17-hadoop3-check/output/Dockerfile |
  | GITHUB PR | https://github.com/apache/hbase/pull/7651 |
  | Optional Tests | javac javadoc unit compile shadedjars |
  | uname | Linux 9b6b1363e88a 6.11.0-1018-azure #18~24.04.1-Ubuntu SMP Sat Jun 28 04:46:03 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux |
  | Build tool | maven |
  | Personality | dev-support/hbase-personality.sh |
  | git revision | master / adfd0560c941109283928946bd671a5836604e46 |
  | Default Java | Eclipse Adoptium-17.0.11+9 |
  | Max. process+thread count | 72 (vs. ulimit of 30000) |
  | modules | C: hbase-examples U: hbase-examples |
  | versions | git=2.34.1 maven=3.9.8 |
  | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org/ |

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@ndimiduk
Copy link
Member Author

Looking over out last successful nightly on master, the Large tests on hbase-server still took 7h. I'm looking for other ways to partition this up.

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from adfd056 to ad51411 Compare January 23, 2026 16:28
@Apache-HBase

This comment has been minimized.

cd "${{ github.workspace }}"
bash src/dev-support/jenkins_precommit_github_yetus.sh

- name: Publish Test Results
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here we should also include a job summary, but do not like other checks, as there is only one unit plugin, showing the result of yetus does not provide more information.

I think we should filter the junit console output, filter out the failed UTs and show them on the job summary page. If no failure, we just show something like 'no failure' or 'pass'.

Copy link
Member Author

Choose a reason for hiding this comment

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

I originally intended to add it later, but yeah, let me look into this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do it later, the current one is OK for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added it but it seems it's not working. I don't see any test reports on the Summary for the failed jobs...

pip install -q -r dev-support/test-waves/requirements.txt

# Fetch wave file - returns path to either GHA artifact cache or committed fallback
WAVE_FILE=$(python dev-support/test-waves/fetch-wave-files.py "${{ matrix.wave }}" "${{ github.base_ref }}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here we could upload the wave files to nightly, which is easier for committers to fix the files manually, and downloading from nightlies does not need any credentials.

And for this job, I prefer we just run it on jenkins, which is easier for us to upload things to nightles, and it does not cost too many resources.

@Apache9
Copy link
Contributor

Apache9 commented Feb 3, 2026

On the small medium tests, I think this is the problem...

[INFO] Apache HBase - Server .............................. SUCCESS [  02:52 h]

Let me check the profile config about parallelism...

@Apache9
Copy link
Contributor

Apache9 commented Feb 3, 2026

We do have some medium tests which need several minutes to finish.

I downloaded the artifacts for small-medium and do some calculating, the total execution time for all the tests is
28404.5s

The execution time for tests which run for more than 90 seconds is
7735.6s

For more than 60 seconds is
11616.4s

And what makes things worse is that, for -PrunLargeTests, all large tests are run in firstPartGroup, where the forkCount is 1C. But for -PrunDevTests, the medium tests are run in secondPartGroup, where the forkCount is 0.5C.

So here, I propose that, first we promote all the tests which run for more than 90 seconds to large tests, and then split small-medium to small and medium, use -PrunSmallTests and -PrunMediumTests, in this way we will all use the 1C forkCount to run the tests, which could make them run faster.

@ndimiduk WDYT?

Thanks.

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from 2d42df9 to c256196 Compare February 3, 2026 10:36
@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

Copy link
Contributor

@Apache9 Apache9 left a comment

Choose a reason for hiding this comment

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

Let's also separated runSmallTests and runMediumTests?

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<includes>
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be overridden by the command line parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay so what I found is, when using the Category selector + the wave include list + the exclude list, all these work together as you would hope: only Large tests are run, which match the include regex, and entries found in the exclude list are indeed excluded.

However, if the yetus feature that adds -Dtest.include.pattern, i think it'll override the includes defined here. I'm not sure if that feature is used... anyway, I think that I prefer these wave definitions live here in the pom, rather than up in the GHA or personality script or whatever.

@Apache9
Copy link
Contributor

Apache9 commented Feb 3, 2026

Seems the include for large wave does not work as expected...

@ndimiduk
Copy link
Member Author

ndimiduk commented Feb 3, 2026

Yeah, pattern matching doesn't support character ranges, or something. Also I noticed that we have some test files named like TestFoo but also FooTest, so addressing that.

I did see your note about breaking up the devTests profile. I'll make that change also.

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from c256196 to 0c5a250 Compare February 3, 2026 15:35
@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@ndimiduk
Copy link
Member Author

ndimiduk commented Feb 3, 2026

I spot-checked the tests run by large waves 1 and 3 (2 is still running) and they look to contain only the expected tests. I think we're good here.

Any other comments @Apache9 ?

@Apache-HBase

This comment has been minimized.

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from 0c5a250 to 4fbb099 Compare February 4, 2026 09:04
@Apache-HBase

This comment has been minimized.

1 similar comment
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

GITHUB_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
GITHUB_USER: ${{ github.actor }}
PATCHDIR: "${{ github.workspace }}/yetus-jdk17-hadoop3-unit-check/output"
PLUGINS: "github,htmlout,maven,unit"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need maven plugin here? We do not need mvninstall check?

Copy link
Member Author

Choose a reason for hiding this comment

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

I dug into this earlier, and the answer seems to be no, the unit plugin has enough to go on, so long as maven plugin is also present. The mvninstall was redundant work from the compile check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean can we remove maven plugin here? The console output for the unit check showed that we did run mvninstall check...

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we need the maven plugin in order for the unit plugin to be able to run maven to run the tests. From reading the yetus code, maven is the "build tool" that is used by the "module worker".

@ndimiduk ndimiduk force-pushed the 29838-jdk-checks-master branch from 4fbb099 to 571a79d Compare February 4, 2026 11:01
@Apache-HBase

This comment has been minimized.

@Apache-HBase
Copy link

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/console in case of problems.

1 similar comment
@Apache-HBase
Copy link

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/console in case of problems.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 31s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+0 🆗 shelldocs 0m 0s Shelldocs was not available.
+0 🆗 yamllint 0m 0s yamllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+1 💚 mvninstall 3m 3s master passed
+1 💚 compile 8m 20s master passed
+1 💚 spotless 0m 43s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 55s the patch passed
+1 💚 compile 8m 26s the patch passed
+1 💚 javac 8m 26s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 pylint 0m 2s No new issues.
+1 💚 shellcheck 0m 0s No new issues.
+1 💚 xmllint 0m 0s No new issues.
+1 💚 hadoopcheck 11m 25s Patch does not cause any errors with Hadoop 3.3.6 3.4.1.
+1 💚 spotless 0m 45s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 11s The patch does not generate ASF License warnings.
43m 47s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #7651
Optional Tests dupname asflicense codespell detsecrets shellcheck shelldocs spotless pylint yamllint javac xmllint hadoopcheck compile
uname Linux c7fb284b04d3 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 571a79d
Default Java Eclipse Adoptium-17.0.11+9
Max. process+thread count 192 (vs. ulimit of 30000)
modules C: . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/console
versions git=2.34.1 maven=3.9.8 pylint=2.15.5 shellcheck=0.8.0 xmllint=20913
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 12s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 2m 17s master passed
+1 💚 compile 1m 39s master passed
+1 💚 javadoc 1m 29s master passed
+1 💚 shadedjars 4m 21s branch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 16s the patch passed
+1 💚 compile 1m 40s the patch passed
+1 💚 javac 1m 40s the patch passed
+1 💚 javadoc 1m 28s the patch passed
+1 💚 shadedjars 4m 19s patch has no errors when building our shaded downstream artifacts.
_ Other Tests _
-1 ❌ unit 226m 29s /patch-unit-root.txt root in the patch failed.
251m 0s
Subsystem Report/Notes
Docker ClientAPI=1.53 ServerAPI=1.53 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #7651
Optional Tests javac javadoc unit shadedjars compile
uname Linux b9cfb4a2d564 6.14.0-1018-aws #18~24.04.1-Ubuntu SMP Mon Nov 24 19:46:27 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 571a79d
Default Java Eclipse Adoptium-17.0.11+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/testReport/
Max. process+thread count 6238 (vs. ulimit of 30000)
modules C: . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7651/18/console
versions git=2.34.1 maven=3.9.8
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

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