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/17 19:03:57 UTC
[hbase-native-client] branch master updated: HBASE-24538: Simplify
the lifecycle of mini cluster. (#7)
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 4f276fb HBASE-24538: Simplify the lifecycle of mini cluster. (#7)
4f276fb is described below
commit 4f276fb99f0c81c786c934635fd0c6ffa324a6e9
Author: Bharath Vissapragada <bh...@apache.org>
AuthorDate: Wed Jun 17 12:02:31 2020 -0700
HBASE-24538: Simplify the lifecycle of mini cluster. (#7)
Cleans up unnecessary methods and moves the teardown of the mini cluster
to the d'tor so that an explicit cleanup is not needed. It is not prone
to the deadlocks mentioned in the jira anymore.
Signed-off-by: Marc Parisi <ph...@apache.org>
---
include/hbase/test-util/mini-cluster.h | 12 ++---
include/hbase/test-util/test-util.h | 5 --
src/hbase/test-util/mini-cluster.cc | 84 ++++++++++++++++------------------
src/hbase/test-util/test-util.cc | 7 ---
4 files changed, 45 insertions(+), 63 deletions(-)
diff --git a/include/hbase/test-util/mini-cluster.h b/include/hbase/test-util/mini-cluster.h
index 425c067..8993bec 100644
--- a/include/hbase/test-util/mini-cluster.h
+++ b/include/hbase/test-util/mini-cluster.h
@@ -26,7 +26,8 @@
namespace hbase {
/**
- * JNI wrapper for the Java based mini cluster implementation.
+ * JNI wrapper for the Java based mini cluster implementation. StartCluster() must be called before calling any other
+ * methods that operate on the JVM, otherwise it triggers an assert that results in a crash.
*/
class MiniCluster {
public:
@@ -39,8 +40,6 @@ class MiniCluster {
const std::vector<std::string> &keys);
jobject CreateTable(const std::string &table, const std::vector<std::string> &families,
const std::vector<std::string> &keys);
- jobject StopRegionServer(int idx);
-
// moves region to server
void MoveRegion(const std::string ®ion, const std::string &server);
// returns the Configuration instance for the cluster
@@ -73,12 +72,13 @@ class MiniCluster {
jmethodID str_ctor_mid_;
jobject htu_;
jobject cluster_;
- std::mutex count_mutex_;
+ // Mutex to be acquired to make changes to the cluster state (start/stop).
+ std::mutex lock_;
+ // To make Setup() calls idempotent.
+ bool inited_ = false;
JavaVM *jvm_;
JNIEnv *CreateVM(JavaVM **jvm);
void Setup();
- jobject htu();
- JNIEnv *env();
jbyteArray StrToByteChar(const std::string &str);
jobject admin();
// Util to get a global ref so that the objected is not GC'ed
diff --git a/include/hbase/test-util/test-util.h b/include/hbase/test-util/test-util.h
index 7c57c28..822124f 100644
--- a/include/hbase/test-util/test-util.h
+++ b/include/hbase/test-util/test-util.h
@@ -37,11 +37,6 @@ class TestUtil {
TestUtil();
/**
- * Destroying a TestUtil will spin down a cluster and remove the test dir.
- */
- ~TestUtil();
-
- /**
* Create a random string. This random string is all letters, as such it is
* very good for use as a directory name.
*/
diff --git a/src/hbase/test-util/mini-cluster.cc b/src/hbase/test-util/mini-cluster.cc
index 971add3..6b237d3 100644
--- a/src/hbase/test-util/mini-cluster.cc
+++ b/src/hbase/test-util/mini-cluster.cc
@@ -23,8 +23,11 @@
#include <fstream>
using hbase::MiniCluster;
-using std::string;
using std::ifstream;
+using std::lock_guard;
+using std::mutex;
+using std::string;
+using std::vector;
JNIEnv *MiniCluster::CreateVM(JavaVM **jvm) {
JavaVMInitArgs args;
@@ -56,7 +59,7 @@ JNIEnv *MiniCluster::CreateVM(JavaVM **jvm) {
}
final_classpath.append(":" + file_contents);
}
- auto options = std::string{"-Djava.class.path="} + final_classpath;
+ auto options = string{"-Djava.class.path="} + final_classpath;
jvm_options.optionString = const_cast<char *>(options.c_str());
args.options = &jvm_options;
args.ignoreUnrecognized = 0;
@@ -70,6 +73,7 @@ MiniCluster::~MiniCluster() {
if (!env_) {
return;
}
+ StopCluster();
// Clean up the global references.
if (testing_util_class_) {
env_->DeleteGlobalRef(testing_util_class_);
@@ -93,7 +97,8 @@ MiniCluster::~MiniCluster() {
}
void MiniCluster::Setup() {
- std::lock_guard<std::mutex> lock(count_mutex_);
+ lock_guard<mutex> lock(lock_);
+ if (inited_) return;
if (!env_) {
env_ = CreateVM(&jvm_);
CHECK_NOTNULL(env_);
@@ -162,19 +167,11 @@ void MiniCluster::Setup() {
if (add_col_mid_ == nullptr) {
LOG(FATAL) << "Couldn't find method addColumn";
}
+ inited_ = true;
}
-jobject MiniCluster::htu() {
- Setup();
- return htu_;
-}
-
-JNIEnv *MiniCluster::env() {
- Setup();
- return env_;
-}
// converts C char* to Java byte[]
-jbyteArray MiniCluster::StrToByteChar(const std::string &str) {
+jbyteArray MiniCluster::StrToByteChar(const string &str) {
if (str.length() == 0) {
return nullptr;
}
@@ -184,7 +181,7 @@ jbyteArray MiniCluster::StrToByteChar(const std::string &str) {
return arr;
}
-jobject MiniCluster::CreateTable(const std::string &table, const std::string &family) {
+jobject MiniCluster::CreateTable(const string &table, const string &family) {
jstring table_name_str = env_->NewStringUTF(table.c_str());
jobject table_name =
env_->CallStaticObjectMethod(table_name_class_, tbl_name_value_of_mid_, table_name_str);
@@ -193,8 +190,7 @@ jobject MiniCluster::CreateTable(const std::string &table, const std::string &fa
return table_obj;
}
-jobject MiniCluster::CreateTable(const std::string &table,
- const std::vector<std::string> &families) {
+jobject MiniCluster::CreateTable(const string &table, const vector<string> &families) {
jstring table_name_str = env_->NewStringUTF(table.c_str());
jobject table_name =
env_->CallStaticObjectMethod(table_name_class_, tbl_name_value_of_mid_, table_name_str);
@@ -209,15 +205,15 @@ jobject MiniCluster::CreateTable(const std::string &table,
return table_obj;
}
-jobject MiniCluster::CreateTable(const std::string &table, const std::string &family,
- const std::vector<std::string> &keys) {
- std::vector<std::string> families{};
+jobject MiniCluster::CreateTable(const string &table, const string &family,
+ const vector<string> &keys) {
+ vector<string> families{};
families.emplace_back(family);
return CreateTable(table, families, keys);
}
-jobject MiniCluster::CreateTable(const std::string &table, const std::vector<std::string> &families,
- const std::vector<std::string> &keys) {
+jobject MiniCluster::CreateTable(const string &table, const vector<string> &families,
+ const vector<string> &keys) {
jstring table_name_str = env_->NewStringUTF(table.c_str());
jobject table_name =
env_->CallStaticObjectMethod(table_name_class_, tbl_name_value_of_mid_, table_name_str);
@@ -225,14 +221,14 @@ jobject MiniCluster::CreateTable(const std::string &table, const std::vector<std
int i = 0;
jobjectArray family_array = env_->NewObjectArray(families.size(), array_element_type, nullptr);
- for (auto family : families) {
+ for (const auto& family : families) {
env_->SetObjectArrayElement(family_array, i++, StrToByteChar(family));
}
jobjectArray key_array = env_->NewObjectArray(keys.size(), array_element_type, nullptr);
i = 0;
- for (auto key : keys) {
+ for (const auto& key : keys) {
env_->SetObjectArrayElement(key_array, i++, StrToByteChar(key));
}
@@ -241,53 +237,40 @@ jobject MiniCluster::CreateTable(const std::string &table, const std::vector<std
return tbl;
}
-jobject MiniCluster::StopRegionServer(int idx) {
- env();
- return env_->CallObjectMethod(cluster_, stop_rs_mid_, (jint)idx);
-}
-
// returns the Configuration for the cluster
jobject MiniCluster::GetConf() {
- env();
+ CHECK_NOTNULL(env_);
return env_->CallObjectMethod(cluster_, get_conf_mid_);
}
// return the Admin instance for the local cluster
jobject MiniCluster::admin() {
- env();
- jobject conn = env_->CallObjectMethod(htu(), get_conn_mid_);
+ CHECK_NOTNULL(env_);
+ jobject conn = env_->CallObjectMethod(htu_, get_conn_mid_);
jobject admin = env_->CallObjectMethod(conn, get_admin_mid_);
return admin;
}
// moves region to server
-void MiniCluster::MoveRegion(const std::string ®ion, const std::string &server) {
+void MiniCluster::MoveRegion(const string ®ion, const string &server) {
+ CHECK_NOTNULL(env_);
jobject admin_ = admin();
env_->CallObjectMethod(admin_, move_mid_, StrToByteChar(region), StrToByteChar(server));
}
jobject MiniCluster::StartCluster(int num_region_servers) {
- env();
+ Setup();
+ CHECK_NOTNULL(env_);
jmethodID mid = env_->GetMethodID(testing_util_class_, "startMiniCluster",
"(I)Lorg/apache/hadoop/hbase/MiniHBaseCluster;");
if (mid == nullptr) {
LOG(INFO) << "Couldn't find method startMiniCluster in the class HBaseTestingUtility";
exit(-1);
}
- cluster_ = env_->CallObjectMethod(htu(), mid, static_cast<jint>(num_region_servers));
+ cluster_ = env_->CallObjectMethod(htu_, mid, static_cast<jint>(num_region_servers));
return cluster_;
}
-void MiniCluster::StopCluster() {
- env();
- jmethodID mid = env_->GetMethodID(testing_util_class_, "shutdownMiniCluster", "()V");
- env_->CallVoidMethod(htu(), mid);
- if (jvm_ != nullptr) {
- jvm_->DestroyJavaVM();
- jvm_ = nullptr;
- }
-}
-
-std::string MiniCluster::GetConfValue(const std::string &key) {
+string MiniCluster::GetConfValue(const string &key) {
jobject conf = GetConf();
auto jval =
(jstring)env_->CallObjectMethod(conf, conf_get_mid_, env_->NewStringUTF(key.c_str()));
@@ -306,3 +289,14 @@ jclass hbase::MiniCluster::FindClassAndGetGlobalRef(const char *cls_name) {
env_->DeleteLocalRef(local_ref);
return static_cast<jclass>(global_ref);
}
+
+void hbase::MiniCluster::StopCluster() {
+ CHECK_NOTNULL(env_);
+ lock_guard<mutex> lock(lock_);
+ if (testing_util_class_ && htu_) {
+ LOG(INFO) << "Shutting down mini cluster.";
+ // Clean up the test cluster.
+ jmethodID mid = env_->GetMethodID(testing_util_class_, "shutdownMiniCluster", "()V");
+ env_->CallVoidMethod(htu_, mid);
+ }
+}
diff --git a/src/hbase/test-util/test-util.cc b/src/hbase/test-util/test-util.cc
index ebaf701..49e7656 100644
--- a/src/hbase/test-util/test-util.cc
+++ b/src/hbase/test-util/test-util.cc
@@ -46,13 +46,6 @@ std::string TestUtil::RandString(int len) {
TestUtil::TestUtil() : temp_dir_(TestUtil::RandString()) {}
-TestUtil::~TestUtil() {
- if (mini_) {
- StopMiniCluster();
- mini_ = nullptr;
- }
-}
-
void TestUtil::StartMiniCluster(int32_t num_region_servers) {
mini_ = std::make_unique<MiniCluster>();
mini_->StartCluster(num_region_servers);