Skip to content

HBASE-24539: Fix the classpath for mini-cluster#4

Merged
bharathv merged 1 commit intoapache:masterfrom
bharathv:HBASE-24539
Jun 12, 2020
Merged

HBASE-24539: Fix the classpath for mini-cluster#4
bharathv merged 1 commit intoapache:masterfrom
bharathv:HBASE-24539

Conversation

@bharathv
Copy link
Contributor

Fixes the classpath and HBaseTestingUtility c'tor invocation.
Promoted some local JNI refs to global to avoid GC cleaning them up.

Tested locally with a single unit-test that can bring-up the minicluster
up cleanly. There are other minicluster lifecycle issues that need to
be fixed (follow on patches) before we can fully use it.

Fixes the classpath and HBaseTestingUtility c'tor invocation.
Promoted some local JNI refs to global to avoid GC cleaning them up.

Tested locally with a single unit-test that can bring-up the minicluster
up cleanly. There are other minicluster lifecycle issues that need to
be fixed (follow on patches) before we can fully use it.
@bharathv bharathv requested a review from joshelser June 11, 2020 19:49
@bharathv
Copy link
Contributor Author

@phrocker FYI

Copy link
Contributor

@phrocker phrocker left a comment

Choose a reason for hiding this comment

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

LGTM. I think I encountered some of those life cycle issues when playing around with this but since I saw your caveat makes sense for follow up. Thanks!

} else {
clspath.assign(classpath, strlen(classpath));
// Default classpath loaded from downloaded HBase src (paths defined in CMakeLists.txt)
string clspath_file_path("./apachehbase-src/hbase-build-configuration/target/cached_classpath.txt");
Copy link
Contributor

Choose a reason for hiding this comment

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

In the case where they provide HBASE_HOME this might get a little tricky. Could have cmake pass HBASE_HOME as an option for compilation here so the path needn't be hardcoded in the future ( as a follow up ).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also takes CLASSPATH env variable (L37). If its set, everything from the classpath is passed over to java.classpath.

CLASSPATH="/path/to/cached_classpath.txt" ./test-binary also works if the users want to pass a custom HBASE_HOME based classpath.

Copy link
Contributor Author

@bharathv bharathv 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 quick review.

} else {
clspath.assign(classpath, strlen(classpath));
// Default classpath loaded from downloaded HBase src (paths defined in CMakeLists.txt)
string clspath_file_path("./apachehbase-src/hbase-build-configuration/target/cached_classpath.txt");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also takes CLASSPATH env variable (L37). If its set, everything from the classpath is passed over to java.classpath.

CLASSPATH="/path/to/cached_classpath.txt" ./test-binary also works if the users want to pass a custom HBASE_HOME based classpath.

@bharathv
Copy link
Contributor Author

I think I encountered some of those life cycle issues

Plan to redo the lifecycle as a part of https://issues.apache.org/jira/browse/HBASE-24538

@bharathv bharathv merged commit 39a06db into apache:master Jun 12, 2020
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.

2 participants