Skip to content
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

HDDS-11347. Add rocks_tools_native lib check in Ozone CLI checknative subcommand #7101

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

smengcl
Copy link
Contributor

@smengcl smengcl commented Aug 20, 2024

What changes were proposed in this pull request?

Display rocks-tools native library loading result in Ozone CLI's checknative subcommand:

$ ./bin/ozone checknative

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11347

How was this patch tested?

# Compile under Linux
mvn clean install -Pdist -DskipTests -Pnative -Drocks_tools_native -e
# Check the output when rocks-tools native lib is loaded
cd hadoop-ozone/dist/target/ozone-*
./bin/ozone checknative

Output (Linux):

Native library checking:
hadoop:  false
ISA-L:   false
rocks-tools: true libozone_rocksdb_tools.so

Output (macOS):

Native library checking:
hadoop:  false 
ISA-L:   false 
rocks-tools: true libozone_rocksdb_tools.dylib

@smengcl smengcl added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Aug 20, 2024
@smengcl smengcl self-assigned this Aug 20, 2024
Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks, @smengcl for the patch. Change looks good to me.

@swamirishi Please take a look as well.

Copy link
Contributor

@swamirishi swamirishi left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @smengcl . Changes look good to me apart from a nitpicky question I had.


// Attempt to load the rocks tools lib
try {
ManagedRawSSTFileReader.loadLibrary();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use NativeLibraryLoader.load(NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME) instead? Since we are checking if the rocks tools library gets loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

@smengcl
Copy link
Contributor Author

smengcl commented Sep 20, 2024

The test failure is weird. I haven't touched anything related to hsync but its failing in one specific test:

https://github.com/apache/ozone/actions/runs/10728014894/job/30403969117?pr=7101#step:6:2770

Error:  Tests run: 41, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 507.786 s <<< FAILURE! - in org.apache.hadoop.fs.ozone.TestHSync
Error:  org.apache.hadoop.fs.ozone.TestHSync.testKeyHSyncThenClose  Time elapsed: 3.83 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
	at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
	at org.apache.hadoop.fs.ozone.TestHSync.testKeyHSyncThenClose(TestHSync.java:378)

And the line number doesn't even point to the right place in the test: Ah, after rebase, now it does

https://github.com/smengcl/hadoop-ozone/blob/HDDS-11347-checknative/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java#L378

Anyway, I've rebased (merged master) it. Let's see how it goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
snapshot https://issues.apache.org/jira/browse/HDDS-6517
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants