You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2023/06/12 06:39:06 UTC
[GitHub] [ozone] adoroszlai opened a new pull request, #4875: HDDS-8817. Reduce effect of native build on total CI time
adoroszlai opened a new pull request, #4875:
URL: https://github.com/apache/ozone/pull/4875
## What changes were proposed in this pull request?
`hdds-rocks-native` library is used from Maven repo, if available, otherwise it is rebuilt from scratch. Rebuild takes ~15 minutes time for each affected check (`build`, `unit`, all splits of `integration`).
This change adds a new `native` check, which builds the native code and runs all unit or integration tests tagged as `@Native`.
Benefits:
1. Checks dependent on `build` are not hindered by the extra time required for native build. `native` check is not on the critical path.
2. `native` check can be skipped for PRs if the `hdds-rocks-native` module is not changed.
https://issues.apache.org/jira/browse/HDDS-8817
## How was this patch tested?
CI for push:
* Native tools built and tested in [`native` check](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5236768539/jobs/9454519333#step:6:753)
* Native build skipped in other checks: [build](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5236768539/jobs/9454519063#step:6:1117), [unit](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5236768539/jobs/9454519375#step:6:899), [integration](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5236768539/jobs/9454519135#step:5:952)
CI for PRs:
* PR with change in native tools [triggered](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5237516920) `native` check
* PR without any change in native tools [skipped](https://github.com/adoroszlai/hadoop-ozone/actions/runs/5237520937) `native` check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4875: HDDS-8817. Create separate check for native build
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1228128468
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdb/util/TestManagedSstFileReader.java:
##########
@@ -53,7 +54,8 @@
/**
* ManagedSstFileReader tests.
*/
-public class TestManagedSstFileReader {
+@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)
Review Comment:
Say if changes happen to ManagedSSTFileReader and not the hdds-rocks-native module directory. Will this particular test class run or not?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4875: HDDS-8817. Reduce effect of native build on total CI time
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4875:
URL: https://github.com/apache/ozone/pull/4875#issuecomment-1586837893
@swamirishi please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4875: HDDS-8817. Create separate check for native build
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1228133451
##########
dev-support/ci/selective_ci_checks.sh:
##########
@@ -434,6 +434,21 @@ function check_needs_findbugs() {
start_end::group_end
}
+function check_needs_native() {
Review Comment:
@adoroszlai Correct me if I am wrong this function will trigger native runs only if the changes are in
`local pattern_array=(
"^hadoop-ozone/dev-support/checks/native.sh"
"^hadoop-hdds/rocks-native"
)` right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4875: HDDS-8817. Create separate check for native build
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1228377799
##########
dev-support/ci/selective_ci_checks.sh:
##########
@@ -434,6 +434,21 @@ function check_needs_findbugs() {
start_end::group_end
}
+function check_needs_native() {
Review Comment:
You are right, it should also run for changes in test tagged as `@Native`, but currently does not. I'll explore that later, but first would like to get this merged. All other CI runs are failing due to the native problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4875: HDDS-8817. Reduce effect of native build on total CI time
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1227086063
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdb/util/TestManagedSstFileReader.java:
##########
@@ -53,7 +54,8 @@
/**
* ManagedSstFileReader tests.
*/
-public class TestManagedSstFileReader {
+@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)
Review Comment:
> this test should just use the jar either from the apache maven repo or the one built in this run
The point of the change is to avoid building the native library for all checks except the `native` one. The `@Native` tag is added to ensure that this test is run in the `native` check and no other checks. All test classes with `loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)` have to be tagged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4875: HDDS-8817. Reduce effect of native build on total CI time
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1226841403
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdb/util/TestManagedSstFileReader.java:
##########
@@ -53,7 +54,8 @@
/**
* ManagedSstFileReader tests.
*/
-public class TestManagedSstFileReader {
+@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)
Review Comment:
@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME) should be only tests inside hdds-rocks-native other modules using this library will have the library from either of the jars.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdb/util/TestManagedSstFileReader.java:
##########
@@ -53,7 +54,8 @@
/**
* ManagedSstFileReader tests.
*/
-public class TestManagedSstFileReader {
+@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)
Review Comment:
The annotation shouldn't be here I believe this test should just use the jar either from the apache maven repo or the one built in this run. I believe we shouldn't tag this test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4875: HDDS-8817. Create separate check for native build
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4875:
URL: https://github.com/apache/ozone/pull/4875#issuecomment-1591074631
Thanks @sodonnel, @smengcl, @swamirishi for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4875: HDDS-8817. Create separate check for native build
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1228129911
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdb/util/TestManagedSstFileReader.java:
##########
@@ -53,7 +54,8 @@
/**
* ManagedSstFileReader tests.
*/
-public class TestManagedSstFileReader {
+@Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)
Review Comment:
We have to run this particular test in every run irrespective of native changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #4875: HDDS-8817. Create separate check for native build
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4875:
URL: https://github.com/apache/ozone/pull/4875
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4875: HDDS-8817. Create separate check for native build
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4875:
URL: https://github.com/apache/ozone/pull/4875#discussion_r1229126673
##########
dev-support/ci/selective_ci_checks.sh:
##########
@@ -434,6 +434,21 @@ function check_needs_findbugs() {
start_end::group_end
}
+function check_needs_native() {
Review Comment:
Updated the patch to run native tests in all modules.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org