From 6e934bf28b05dbcdc33318725eab8fefc7857a41 Mon Sep 17 00:00:00 2001 From: Sudeep Sunthankar Date: Wed, 18 Jan 2017 20:25:18 +1100 Subject: [PATCH] Passing instance of Configuration to LocationCache so that we dont have to hardcode values for Zookeeper Quorum and Zookeeper Znode. diff --git a/hbase-native-client/core/location-cache-test.cc b/hbase-native-client/core/location-cache-test.cc index 53db6fc..dc3b4f3 100644 --- a/hbase-native-client/core/location-cache-test.cc +++ b/hbase-native-client/core/location-cache-test.cc @@ -23,18 +23,85 @@ #include +#include "core/hbase_configuration_loader.h" #include "if/HBase.pb.h" #include "serde/table-name.h" #include "test-util/test-util.h" + using namespace hbase; using namespace std::chrono; +class LocationCacheTest { + public: + const static std::string kDefHBaseConfPath; + + const static std::string kHBaseDefaultXml; + const static std::string kHBaseSiteXml; + + const static std::string kHBaseXmlData; + + static void WriteDataToFile(const std::string &file, const std::string &xml_data) { + std::ofstream hbase_conf; + hbase_conf.open(file.c_str()); + hbase_conf << xml_data; + hbase_conf.close(); + } + + static void CreateHBaseConf(const std::string &dir, const std::string &file, + const std::string xml_data) { + // Directory will be created if not present + if (!boost::filesystem::exists(dir)) { + boost::filesystem::create_directories(dir); + } + // Remove temp file always + boost::filesystem::remove((dir + file).c_str()); + WriteDataToFile((dir + file), xml_data); + } + + static void CreateHBaseConfWithEnv() { + // Creating Empty Config Files so that we dont get a Configuration exception @Client + CreateHBaseConf(kDefHBaseConfPath, kHBaseDefaultXml, kHBaseXmlData); + CreateHBaseConf(kDefHBaseConfPath, kHBaseSiteXml, kHBaseXmlData); + setenv("HBASE_CONF", kDefHBaseConfPath.c_str(), 1); + } +}; + +const std::string LocationCacheTest::kDefHBaseConfPath("./build/test-data/client-test/conf/"); + +const std::string LocationCacheTest::kHBaseDefaultXml("hbase-default.xml"); +const std::string LocationCacheTest::kHBaseSiteXml("hbase-site.xml"); + +const std::string LocationCacheTest::kHBaseXmlData( + "\n\n\n\n\n"); + TEST(LocationCacheTest, TestGetMetaNodeContents) { + unsetenv("HBASE_CONF"); + LocationCacheTest::CreateHBaseConfWithEnv(); + // Create Configuration + hbase::HBaseConfigurationLoader loader; + auto conf = loader.LoadDefaultResources(); + ASSERT_TRUE(conf) << "No configuration object present."; + auto pconf = std::make_shared(conf.value()); + TestUtil test_util{}; auto cpu = std::make_shared(4); auto io = std::make_shared(4); - LocationCache cache{"localhost:2181", cpu, io}; + LocationCache cache{pconf, cpu, io}; auto f = cache.LocateMeta(); auto result = f.get(); ASSERT_FALSE(f.hasException()); @@ -43,11 +110,18 @@ TEST(LocationCacheTest, TestGetMetaNodeContents) { } TEST(LocationCacheTest, TestGetRegionLocation) { + unsetenv("HBASE_CONF"); + LocationCacheTest::CreateHBaseConfWithEnv(); + // Create Configuration + hbase::HBaseConfigurationLoader loader; + auto conf = loader.LoadDefaultResources(); + ASSERT_TRUE(conf) << "No configuration object present."; + auto pconf = std::make_shared(conf.value()); + TestUtil test_util{}; auto cpu = std::make_shared(4); auto io = std::make_shared(4); - LocationCache cache{"localhost:2181", cpu, io}; - + LocationCache cache{pconf, cpu, io}; // If there is no table this should throw an exception auto tn = folly::to("t"); auto row = "test"; diff --git a/hbase-native-client/core/location-cache.cc b/hbase-native-client/core/location-cache.cc index 6c2a790..da77849 100644 --- a/hbase-native-client/core/location-cache.cc +++ b/hbase-native-client/core/location-cache.cc @@ -50,20 +50,18 @@ using hbase::pb::ServerName; using hbase::pb::MetaRegionServer; using hbase::pb::RegionInfo; -// TODO(eclark): make this configurable on client creation -static const char META_ZNODE_NAME[] = "/hbase/meta-region-server"; - -LocationCache::LocationCache(std::string quorum_spec, +LocationCache::LocationCache(std::shared_ptr conf, std::shared_ptr cpu_executor, std::shared_ptr io_executor) - : quorum_spec_(quorum_spec), + : conf_(conf), cpu_executor_(cpu_executor), meta_promise_(nullptr), meta_lock_(), cp_(io_executor), meta_util_(), zk_(nullptr) { - zk_ = zookeeper_init(quorum_spec.c_str(), nullptr, 1000, 0, 0, 0); + quorum_spec_ = conf_->Get(kHBaseZookeeperQuorum_, kDefHBaseZookeeperQuorum_); + zk_ = zookeeper_init(quorum_spec_.c_str(), nullptr, 1000, 0, 0, 0); } LocationCache::~LocationCache() { @@ -90,7 +88,9 @@ void LocationCache::InvalidateMeta() { /// MUST hold the meta_lock_ void LocationCache::RefreshMetaLocation() { meta_promise_ = make_unique>(); - cpu_executor_->add([&] { meta_promise_->setWith([&] { return this->ReadMetaLocation(); }); }); + cpu_executor_->add([&] { + meta_promise_->setWith([&] { return this->ReadMetaLocation(); }); + }); } ServerName LocationCache::ReadMetaLocation() { @@ -99,8 +99,10 @@ ServerName LocationCache::ReadMetaLocation() { // This needs to be int rather than size_t as that's what ZK expects. int len = buf->capacity(); + std::string zk_node = conf_->Get(kHBaseMetaZnodeName_, kDefHBaseMetaZnodeName_); + zk_node += kHBaseMetaRegionServer_; // TODO(elliott): handle disconnects/reconntion as needed. - int zk_result = zoo_get(this->zk_, META_ZNODE_NAME, 0, + int zk_result = zoo_get(this->zk_, zk_node.c_str(), 0, reinterpret_cast(buf->writableData()), &len, nullptr); if (zk_result != ZOK || len < 9) { LOG(ERROR) << "Error getting meta location."; @@ -140,8 +142,8 @@ Future> LocationCache::LocateFromMeta(const Tabl return rl; }) .then([this](std::shared_ptr rl) { - auto remote_id = - std::make_shared(rl->server_name().host_name(), rl->server_name().port()); + auto remote_id = std::make_shared(rl->server_name().host_name(), + rl->server_name().port()); // Now fill out the connection. rl->set_service(cp_.GetConnection(remote_id)->get_service()); return rl; diff --git a/hbase-native-client/core/location-cache.h b/hbase-native-client/core/location-cache.h index b290a1f..1988e53 100644 --- a/hbase-native-client/core/location-cache.h +++ b/hbase-native-client/core/location-cache.h @@ -30,6 +30,7 @@ #include #include "connection/connection-pool.h" +#include "core/configuration.h" #include "core/meta-utils.h" #include "core/region-location.h" #include "serde/table-name.h" @@ -49,12 +50,12 @@ class LocationCache { public: /** * Constructor. - * @param quorum_spec Where to connect for Zookeeper. + * @param conf Configuration instance to fetch Zookeeper Quorum and Zookeeper Znode. * @param cpu_executor executor used to run non network IO based * continuations. * @param io_executor executor used to talk to the network */ - LocationCache(std::string quorum_spec, + LocationCache(std::shared_ptr conf, std::shared_ptr cpu_executor, std::shared_ptr io_executor); /** @@ -79,7 +80,7 @@ class LocationCache { * @param row of the table to look up. This object must live until after the * future is returned */ - folly::Future> LocateFromMeta(const hbase::pb::TableName &tn, + folly::Future > LocateFromMeta(const hbase::pb::TableName &tn, const std::string &row); /** @@ -95,12 +96,18 @@ class LocationCache { /* data */ std::string quorum_spec_; std::shared_ptr cpu_executor_; - std::unique_ptr> meta_promise_; + std::unique_ptr > meta_promise_; std::mutex meta_lock_; MetaUtil meta_util_; ConnectionPool cp_; // TODO: migrate this to a smart pointer with a deleter. zhandle_t *zk_; + std::shared_ptr conf_; + const std::string kHBaseZookeeperQuorum_ = "hbase.zookeeper.quorum"; + const std::string kDefHBaseZookeeperQuorum_ = "localhost:2181"; + const std::string kHBaseMetaZnodeName_ = "zookeeper.znode.parent"; + const std::string kDefHBaseMetaZnodeName_ = "/hbase"; + const std::string kHBaseMetaRegionServer_ = "/meta-region-server"; }; } // namespace hbase -- 1.8.3.1