You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bh...@apache.org on 2020/06/12 22:12:52 UTC
[hbase-native-client] branch master updated: HBASE-24539: Fix the
classpath for mini-cluster (#4)
This is an automated email from the ASF dual-hosted git repository.
bharathv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase-native-client.git
The following commit(s) were added to refs/heads/master by this push:
new 39a06db HBASE-24539: Fix the classpath for mini-cluster (#4)
39a06db is described below
commit 39a06db6e7abcfc9f767d910d59cac2b52c17746
Author: Bharath Vissapragada <bh...@apache.org>
AuthorDate: Fri Jun 12 15:12:43 2020 -0700
HBASE-24539: Fix the classpath for mini-cluster (#4)
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.
Signed-off-by: Marc Parisi <ph...@apache.org>
---
include/hbase/test-util/mini-cluster.h | 7 +-
src/hbase/test-util/mini-cluster.cc | 274 ++++++++++++++++-----------------
2 files changed, 141 insertions(+), 140 deletions(-)
diff --git a/include/hbase/test-util/mini-cluster.h b/include/hbase/test-util/mini-cluster.h
index 978ded8..425c067 100644
--- a/include/hbase/test-util/mini-cluster.h
+++ b/include/hbase/test-util/mini-cluster.h
@@ -25,6 +25,9 @@
namespace hbase {
+/**
+ * JNI wrapper for the Java based mini cluster implementation.
+ */
class MiniCluster {
public:
virtual ~MiniCluster();
@@ -43,7 +46,7 @@ class MiniCluster {
// returns the Configuration instance for the cluster
jobject GetConf();
// returns the value for config key retrieved from cluster
- const std::string GetConfValue(const std::string &key);
+ std::string GetConfValue(const std::string &key);
private:
JNIEnv *env_;
@@ -78,5 +81,7 @@ class MiniCluster {
JNIEnv *env();
jbyteArray StrToByteChar(const std::string &str);
jobject admin();
+ // Util to get a global ref so that the objected is not GC'ed
+ jclass FindClassAndGetGlobalRef(const char* cls_name);
};
} /*namespace hbase*/
diff --git a/src/hbase/test-util/mini-cluster.cc b/src/hbase/test-util/mini-cluster.cc
index 0e749c6..971add3 100644
--- a/src/hbase/test-util/mini-cluster.cc
+++ b/src/hbase/test-util/mini-cluster.cc
@@ -20,163 +20,147 @@
#include "hbase/test-util/mini-cluster.h"
#include <fcntl.h>
#include <glog/logging.h>
-#include <boost/filesystem/fstream.hpp>
-#include <boost/filesystem/operations.hpp>
#include <fstream>
using hbase::MiniCluster;
+using std::string;
+using std::ifstream;
JNIEnv *MiniCluster::CreateVM(JavaVM **jvm) {
JavaVMInitArgs args;
JavaVMOption jvm_options;
args.version = JNI_VERSION_1_6;
args.nOptions = 1;
+ // Sets the Java classpath to load the test jars and dependencies.
+ // Either set the CLASSPATH environment variable before running the test or the test loads it from the default
+ // location, if it exists.
char *classpath = getenv("CLASSPATH");
- std::string clspath;
- if (classpath == NULL || strstr(classpath, "-tests.jar") == NULL) {
- std::string clsPathFilePath("../../../hbase-build-configuration/target/cached_classpath.txt");
- std::ifstream fd(clsPathFilePath);
- std::string prefix("");
- if (fd.is_open()) {
- if (classpath == NULL) {
- LOG(INFO) << "got empty classpath";
- } else {
- // prefix bootstrapper.jar
- prefix.assign(classpath);
- }
- std::string line;
- if (getline(fd, line)) {
- clspath = prefix + ":" + line;
- int ret = setenv("CLASSPATH", clspath.c_str(), 1);
- LOG(INFO) << "set clspath " << ret;
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
- }
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
+ // Copy it to a string so that we don't inadverdently change the environment variable.
+ string final_classpath;
+ if (classpath == nullptr || strstr(classpath, "-tests.jar") == nullptr) {
+ if (classpath != nullptr) {
+ final_classpath.assign(classpath);
}
- fd.close();
- } 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");
+ ifstream fd(clspath_file_path);
+ if (!fd.is_open()) {
+ LOG(FATAL) << "No valid classpath found. If you haven't built with DOWNLOAD_DEPENDENCIES=ON, set the appropriate"
+ "CLASSPATH variable pointing to the content of cached_classpath.txt generated from compiling HBase"
+ "sources.";
+ }
+ string file_contents;
+ getline(fd, file_contents);
+ if (file_contents.empty()) {
+ LOG(FATAL) << "Empty classpath file encountered: " << clspath_file_path;
+ }
+ final_classpath.append(":" + file_contents);
}
-
- auto options = std::string{"-Djava.class.path="} + clspath;
+ auto options = std::string{"-Djava.class.path="} + final_classpath;
jvm_options.optionString = const_cast<char *>(options.c_str());
args.options = &jvm_options;
args.ignoreUnrecognized = 0;
- int rv;
- rv = JNI_CreateJavaVM(jvm, reinterpret_cast<void **>(&env_), &args);
- CHECK(rv >= 0 && env_);
+ int rv = JNI_CreateJavaVM(jvm, reinterpret_cast<void **>(&env_), &args);
+ CHECK_NOTNULL(env_);
+ CHECK(rv >= 0);
return env_;
}
MiniCluster::~MiniCluster() {
- if (jvm_ != NULL) {
+ if (!env_) {
+ return;
+ }
+ // Clean up the global references.
+ if (testing_util_class_) {
+ env_->DeleteGlobalRef(testing_util_class_);
+ }
+ if (table_name_class_) {
+ env_->DeleteGlobalRef(table_name_class_);
+ }
+ if (conf_class_) {
+ env_->DeleteGlobalRef(conf_class_);
+ }
+ if (put_class_) {
+ env_->DeleteGlobalRef(put_class_);
+ }
+ if (htu_) {
+ env_->DeleteGlobalRef(htu_);
+ }
+ if (jvm_ != nullptr) {
jvm_->DestroyJavaVM();
- jvm_ = NULL;
+ jvm_ = nullptr;
}
- env_ = nullptr;
}
void MiniCluster::Setup() {
- jmethodID constructor;
std::lock_guard<std::mutex> lock(count_mutex_);
- if (env_ == NULL) {
+ if (!env_) {
env_ = CreateVM(&jvm_);
- if (env_ == NULL) {
- exit(-1);
- }
- testing_util_class_ = env_->FindClass("org/apache/hadoop/hbase/HBaseTestingUtility");
- // this should be converted to a globalref I think to avoid the underlying java obj getting
- // GC'ed
- if (testing_util_class_ == NULL) {
- LOG(INFO) << "Couldn't find class HBaseTestingUtility";
- exit(-1);
- }
- jmethodID mid = env_->GetStaticMethodID(testing_util_class_, "createLocalHTU",
- "()Lorg/apache/hadoop/hbase/HBaseTestingUtility;");
- htu_ = env_->CallStaticObjectMethod(testing_util_class_, mid);
- // this should be converted to a globalref I think to avoid the underlying java obj getting
- // GC'ed
- if (htu_ == NULL) {
- LOG(INFO) << "Couldn't invoke method createLocalHTU in HBaseTestingUtility";
- exit(-1);
- }
- get_conn_mid_ = env_->GetMethodID(testing_util_class_, "getConnection",
- "()Lorg/apache/hadoop/hbase/client/Connection;");
- jclass conn_class = env_->FindClass("org/apache/hadoop/hbase/client/Connection");
- get_admin_mid_ =
- env_->GetMethodID(conn_class, "getAdmin", "()Lorg/apache/hadoop/hbase/client/Admin;");
- get_table_mid_ = env_->GetMethodID(
- conn_class, "getTable",
- "(Lorg/apache/hadoop/hbase/TableName;)Lorg/apache/hadoop/hbase/client/Table;");
- if (get_table_mid_ == NULL) {
- LOG(INFO) << "Couldn't find getConnection";
- exit(-1);
- }
- jclass adminClass = env_->FindClass("org/apache/hadoop/hbase/client/Admin");
- move_mid_ = env_->GetMethodID(adminClass, "move", "([B[B)V");
- if (move_mid_ == NULL) {
- LOG(INFO) << "Couldn't find move";
- exit(-1);
- }
- create_table_mid_ =
- env_->GetMethodID(testing_util_class_, "createTable",
- "(Lorg/apache/hadoop/hbase/TableName;Ljava/lang/String;)Lorg/"
- "apache/hadoop/hbase/client/Table;");
- create_table_families_mid_ = env_->GetMethodID(testing_util_class_, "createTable",
- "(Lorg/apache/hadoop/hbase/TableName;[[B)Lorg/"
- "apache/hadoop/hbase/client/Table;");
- create_table_with_split_mid_ = env_->GetMethodID(
- testing_util_class_, "createTable",
- "(Lorg/apache/hadoop/hbase/TableName;[[B[[B)Lorg/apache/hadoop/hbase/client/Table;");
- if (create_table_with_split_mid_ == NULL) {
- LOG(INFO) << "Couldn't find method createTable with split";
- exit(-1);
- }
-
- table_name_class_ = env_->FindClass("org/apache/hadoop/hbase/TableName");
- tbl_name_value_of_mid_ = env_->GetStaticMethodID(
- table_name_class_, "valueOf", "(Ljava/lang/String;)Lorg/apache/hadoop/hbase/TableName;");
- if (tbl_name_value_of_mid_ == NULL) {
- LOG(INFO) << "Couldn't find method valueOf in TableName";
- exit(-1);
- }
- jclass hbaseMiniClusterClass = env_->FindClass("org/apache/hadoop/hbase/MiniHBaseCluster");
- stop_rs_mid_ =
- env_->GetMethodID(hbaseMiniClusterClass, "stopRegionServer",
- "(I)Lorg/apache/hadoop/hbase/util/JVMClusterUtil$RegionServerThread;");
- get_conf_mid_ = env_->GetMethodID(hbaseMiniClusterClass, "getConfiguration",
- "()Lorg/apache/hadoop/conf/Configuration;");
-
- conf_class_ = env_->FindClass("org/apache/hadoop/conf/Configuration");
- set_conf_mid_ =
- env_->GetMethodID(conf_class_, "set", "(Ljava/lang/String;Ljava/lang/String;)V");
- if (set_conf_mid_ == NULL) {
- LOG(INFO) << "Couldn't find method getConf in MiniHBaseCluster";
- exit(-1);
- }
- conf_get_mid_ = env_->GetMethodID(conf_class_, "get", "(Ljava/lang/String;)Ljava/lang/String;");
-
- jclass tableClass = env_->FindClass("org/apache/hadoop/hbase/client/Table");
- put_mid_ = env_->GetMethodID(tableClass, "put", "(Lorg/apache/hadoop/hbase/client/Put;)V");
- jclass connFactoryClass = env_->FindClass("org/apache/hadoop/hbase/client/ConnectionFactory");
- create_conn_mid_ = env_->GetStaticMethodID(connFactoryClass, "createConnection",
- "()Lorg/apache/hadoop/hbase/client/Connection;");
- if (create_conn_mid_ == NULL) {
- LOG(INFO) << "Couldn't find createConnection";
- exit(-1);
- }
- put_class_ = env_->FindClass("org/apache/hadoop/hbase/client/Put");
- put_ctor_ = env_->GetMethodID(put_class_, "<init>", "([B)V");
- add_col_mid_ =
- env_->GetMethodID(put_class_, "addColumn", "([B[B[B)Lorg/apache/hadoop/hbase/client/Put;");
- if (add_col_mid_ == NULL) {
- LOG(INFO) << "Couldn't find method addColumn";
- exit(-1);
- }
+ CHECK_NOTNULL(env_);
+ }
+ testing_util_class_ = FindClassAndGetGlobalRef("org/apache/hadoop/hbase/HBaseTestingUtility");
+ jmethodID mid = env_->GetMethodID(testing_util_class_, "<init>","()V");
+ CHECK_NOTNULL(mid);
+ jobject local_htu = env_->NewObject(testing_util_class_, mid);
+ if (!local_htu) {
+ LOG(FATAL) << "Couldn't invoke method createLocalHTU in HBaseTestingUtility";
+ }
+ htu_ = env_->NewGlobalRef(local_htu);
+ env_->DeleteLocalRef(local_htu);
+ get_conn_mid_ = env_->GetMethodID(testing_util_class_, "getConnection",
+ "()Lorg/apache/hadoop/hbase/client/Connection;");
+ jclass conn_class = env_->FindClass("org/apache/hadoop/hbase/client/Connection");
+ get_admin_mid_ =
+ env_->GetMethodID(conn_class, "getAdmin", "()Lorg/apache/hadoop/hbase/client/Admin;");
+ get_table_mid_ = env_->GetMethodID(conn_class, "getTable",
+ "(Lorg/apache/hadoop/hbase/TableName;)Lorg/apache/hadoop/hbase/client/Table;");
+ if (!get_table_mid_) {
+ LOG(FATAL) << "Couldn't find getConnection";
+ }
+ jclass adminClass = env_->FindClass("org/apache/hadoop/hbase/client/Admin");
+ move_mid_ = env_->GetMethodID(adminClass, "move", "([B[B)V");
+ if (move_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find move";
+ }
+ create_table_mid_ = env_->GetMethodID(testing_util_class_, "createTable",
+ "(Lorg/apache/hadoop/hbase/TableName;Ljava/lang/String;)Lorg/apache/hadoop/hbase/client/Table;");
+ create_table_families_mid_ = env_->GetMethodID(testing_util_class_, "createTable",
+ "(Lorg/apache/hadoop/hbase/TableName;[[B)Lorg/apache/hadoop/hbase/client/Table;");
+ create_table_with_split_mid_ = env_->GetMethodID(testing_util_class_, "createTable",
+ "(Lorg/apache/hadoop/hbase/TableName;[[B[[B)Lorg/apache/hadoop/hbase/client/Table;");
+ if (create_table_with_split_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find method createTable with split";
+ }
+ table_name_class_ = FindClassAndGetGlobalRef("org/apache/hadoop/hbase/TableName");
+ tbl_name_value_of_mid_ = env_->GetStaticMethodID(
+ table_name_class_, "valueOf", "(Ljava/lang/String;)Lorg/apache/hadoop/hbase/TableName;");
+ if (tbl_name_value_of_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find method valueOf in TableName";
+ }
+ jclass hbaseMiniClusterClass = env_->FindClass("org/apache/hadoop/hbase/MiniHBaseCluster");
+ stop_rs_mid_ =env_->GetMethodID(hbaseMiniClusterClass, "stopRegionServer",
+ "(I)Lorg/apache/hadoop/hbase/util/JVMClusterUtil$RegionServerThread;");
+ get_conf_mid_ = env_->GetMethodID(hbaseMiniClusterClass, "getConfiguration",
+ "()Lorg/apache/hadoop/conf/Configuration;");
+ conf_class_ = FindClassAndGetGlobalRef("org/apache/hadoop/conf/Configuration");
+ set_conf_mid_ = env_->GetMethodID(conf_class_, "set", "(Ljava/lang/String;Ljava/lang/String;)V");
+ if (set_conf_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find method getConf in MiniHBaseCluster";
+ }
+ conf_get_mid_ = env_->GetMethodID(conf_class_, "get", "(Ljava/lang/String;)Ljava/lang/String;");
+ jclass tableClass = env_->FindClass("org/apache/hadoop/hbase/client/Table");
+ put_mid_ = env_->GetMethodID(tableClass, "put", "(Lorg/apache/hadoop/hbase/client/Put;)V");
+ jclass connFactoryClass = env_->FindClass("org/apache/hadoop/hbase/client/ConnectionFactory");
+ create_conn_mid_ = env_->GetStaticMethodID(connFactoryClass, "createConnection",
+ "()Lorg/apache/hadoop/hbase/client/Connection;");
+ if (create_conn_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find createConnection";
+ }
+ put_class_ = FindClassAndGetGlobalRef("org/apache/hadoop/hbase/client/Put");
+ put_ctor_ = env_->GetMethodID(put_class_, "<init>", "([B)V");
+ add_col_mid_ = env_->GetMethodID(put_class_, "addColumn", "([B[B[B)Lorg/apache/hadoop/hbase/client/Put;");
+ if (add_col_mid_ == nullptr) {
+ LOG(FATAL) << "Couldn't find method addColumn";
}
}
@@ -217,7 +201,7 @@ jobject MiniCluster::CreateTable(const std::string &table,
jclass array_element_type = env_->FindClass("[B");
jobjectArray family_array = env_->NewObjectArray(families.size(), array_element_type, nullptr);
int i = 0;
- for (auto family : families) {
+ for (const auto& family : families) {
env_->SetObjectArrayElement(family_array, i++, StrToByteChar(family));
}
jobject table_obj =
@@ -228,7 +212,7 @@ jobject MiniCluster::CreateTable(const std::string &table,
jobject MiniCluster::CreateTable(const std::string &table, const std::string &family,
const std::vector<std::string> &keys) {
std::vector<std::string> families{};
- families.push_back(std::string{family});
+ families.emplace_back(family);
return CreateTable(table, families, keys);
}
@@ -284,8 +268,8 @@ void MiniCluster::MoveRegion(const std::string ®ion, const std::string &serve
jobject MiniCluster::StartCluster(int num_region_servers) {
env();
jmethodID mid = env_->GetMethodID(testing_util_class_, "startMiniCluster",
- "(I)Lorg/apache/hadoop/hbase/MiniHBaseCluster;");
- if (mid == NULL) {
+ "(I)Lorg/apache/hadoop/hbase/MiniHBaseCluster;");
+ if (mid == nullptr) {
LOG(INFO) << "Couldn't find method startMiniCluster in the class HBaseTestingUtility";
exit(-1);
}
@@ -297,16 +281,28 @@ void MiniCluster::StopCluster() {
env();
jmethodID mid = env_->GetMethodID(testing_util_class_, "shutdownMiniCluster", "()V");
env_->CallVoidMethod(htu(), mid);
- if (jvm_ != NULL) {
+ if (jvm_ != nullptr) {
jvm_->DestroyJavaVM();
- jvm_ = NULL;
+ jvm_ = nullptr;
}
}
-const std::string MiniCluster::GetConfValue(const std::string &key) {
+std::string MiniCluster::GetConfValue(const std::string &key) {
jobject conf = GetConf();
- jstring jval =
+ auto jval =
(jstring)env_->CallObjectMethod(conf, conf_get_mid_, env_->NewStringUTF(key.c_str()));
const char *val = env_->GetStringUTFChars(jval, 0);
return val;
}
+
+jclass hbase::MiniCluster::FindClassAndGetGlobalRef(const char *cls_name) {
+ // Make sure env is initialized.
+ CHECK_NOTNULL(env_);
+ jclass local_ref = env_->FindClass(cls_name);
+ if (!local_ref) {
+ LOG(FATAL) << "Couldn't find class: " << cls_name;
+ }
+ jobject global_ref = env_->NewGlobalRef(local_ref);
+ env_->DeleteLocalRef(local_ref);
+ return static_cast<jclass>(global_ref);
+}