kudu git commit: Extract connection retrying and HA support from HmsCatalog
Repository: kudu Updated Branches: refs/heads/master f1f192ca3 -> 42b75278a Extract connection retrying and HA support from HmsCatalog Introduces a new abstraction, thrift::HaClient, for providing HA and retrying support on top of Thrift clients like HmsClient and SentryClient. The implementation is extracted from HmsCatalog, and is not changed in any significant ways. The implementation is inlined into a header because the class requires a template parameter (to choose between HmsClient or SentryClient), and forward declaring these instantiations is not possible because the thrift module does not link to the hms or sentry modules. No tests are provided for HaClient in isolation, instead it's tested through existing and new failover tests of HaClient and HaClient. This patch should have little or no functional changes. Change-Id: Id8135f4c3995bba0ba28384c35696a1771ff5296 Reviewed-on: http://gerrit.cloudera.org:8080/11570 Reviewed-by: Adar Dembo Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/42b75278 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/42b75278 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/42b75278 Branch: refs/heads/master Commit: 42b75278a8d226b0235736aae735edeeeb55ab21 Parents: f1f192c Author: Dan Burkert Authored: Tue Oct 2 14:05:00 2018 -0700 Committer: Dan Burkert Committed: Fri Oct 5 21:42:43 2018 + -- src/kudu/hms/hms_catalog.cc | 216 src/kudu/hms/hms_catalog.h| 33 +--- src/kudu/hms/hms_client.cc| 2 + src/kudu/hms/hms_client.h | 23 +-- src/kudu/sentry/sentry_client-test.cc | 29 +++- src/kudu/sentry/sentry_client.cc | 2 + src/kudu/sentry/sentry_client.h | 13 +- src/kudu/thrift/client.cc | 16 +- src/kudu/thrift/client.h | 256 - 9 files changed, 343 insertions(+), 247 deletions(-) -- http://git-wip-us.apache.org/repos/asf/kudu/blob/42b75278/src/kudu/hms/hms_catalog.cc -- diff --git a/src/kudu/hms/hms_catalog.cc b/src/kudu/hms/hms_catalog.cc index ae4d998..7620b2e 100644 --- a/src/kudu/hms/hms_catalog.cc +++ b/src/kudu/hms/hms_catalog.cc @@ -17,8 +17,6 @@ #include "kudu/hms/hms_catalog.h" -#include -#include #include #include #include @@ -43,11 +41,10 @@ #include "kudu/hms/hive_metastore_types.h" #include "kudu/hms/hms_client.h" #include "kudu/thrift/client.h" -#include "kudu/util/async_util.h" #include "kudu/util/flag_tags.h" +#include "kudu/util/monotime.h" #include "kudu/util/net/net_util.h" #include "kudu/util/slice.h" -#include "kudu/util/threadpool.h" using boost::none; using boost::optional; @@ -123,12 +120,7 @@ const char* const HmsCatalog::kInvalidTableError = "when the Hive Metastore inte "identifier pair, each containing only ASCII alphanumeric characters, '_', and '/'"; HmsCatalog::HmsCatalog(string master_addresses) -: master_addresses_(std::move(master_addresses)), - hms_client_(HostPort("", 0), thrift::ClientOptions()), - reconnect_after_(MonoTime::Now()), - reconnect_failure_(Status::OK()), - consecutive_reconnect_failures_(0), - reconnect_idx_(0) { +: master_addresses_(std::move(master_addresses)) { } HmsCatalog::~HmsCatalog() { @@ -136,35 +128,32 @@ HmsCatalog::~HmsCatalog() { } Status HmsCatalog::Start() { - if (threadpool_) { -return Status::IllegalState("HMS Catalog is already started"); - } - - RETURN_NOT_OK(ParseUris(FLAGS_hive_metastore_uris, _addresses_)); + vector addresses; + RETURN_NOT_OK(ParseUris(FLAGS_hive_metastore_uris, )); - // The thread pool must be capped at one thread to ensure serialized access to - // the fields of HmsCatalog. - RETURN_NOT_OK(ThreadPoolBuilder("hms-catalog") - .set_min_threads(1) - .set_max_threads(1) - .Build(_)); + thrift::ClientOptions options; + options.send_timeout = MonoDelta::FromSeconds(FLAGS_hive_metastore_send_timeout); + options.recv_timeout = MonoDelta::FromSeconds(FLAGS_hive_metastore_recv_timeout); + options.conn_timeout = MonoDelta::FromSeconds(FLAGS_hive_metastore_conn_timeout); + options.enable_kerberos = FLAGS_hive_metastore_sasl_enabled; + options.service_principal = FLAGS_hive_metastore_kerberos_principal; + options.max_buf_size = FLAGS_hive_metastore_max_message_size; + options.retry_count = FLAGS_hive_metastore_retry_count; - return Status::OK(); + return ha_client_.Start(std::move(addresses), std::move(options)); } void HmsCatalog::Stop() { - if (threadpool_) { -threadpool_->Shutdown(); - } + ha_client_.Stop(); } Status HmsCatalog::CreateTable(const string& id,
kudu git commit: KUDU-1592 Make warnings about FBM more ominous
Repository: kudu Updated Branches: refs/heads/master b848488d9 -> f1f192ca3 KUDU-1592 Make warnings about FBM more ominous The documentation warned about scalability issues with file block manager, but it was rather lightly worded and so was the warning message of a hole punch test failure. This commit removes the actual flag necessary to enable FBM from the hole punching tests' warning to force users look it up in the documentation instead of blindly applying it, adds a note about it being unsuitable for production use to the flag's description and expands the warning in the troubleshooting section. Change-Id: I15adf42dd7e6376a85cdb178ad460f239a4f87a1 Reviewed-on: http://gerrit.cloudera.org:8080/11579 Reviewed-by: Adar Dembo Reviewed-by: Andrew Wong Tested-by: Attila Bukor Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f1f192ca Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f1f192ca Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f1f192ca Branch: refs/heads/master Commit: f1f192ca3abead4055e7ab6e6bba84d3d6e9866b Parents: b848488 Author: Attila Bukor Authored: Fri Oct 5 19:53:38 2018 +0200 Committer: Attila Bukor Committed: Fri Oct 5 18:40:27 2018 + -- docs/troubleshooting.adoc | 26 -- src/kudu/fs/data_dirs.cc | 8 ++-- src/kudu/fs/fs_manager.cc | 3 ++- 3 files changed, 28 insertions(+), 9 deletions(-) -- http://git-wip-us.apache.org/repos/asf/kudu/blob/f1f192ca/docs/troubleshooting.adoc -- diff --git a/docs/troubleshooting.adoc b/docs/troubleshooting.adoc index 1bebe07..c3f8fbf 100644 --- a/docs/troubleshooting.adoc +++ b/docs/troubleshooting.adoc @@ -48,8 +48,12 @@ Error during hole punch test. The log block manager requires a filesystem with hole punching support such as ext4 or xfs. On el6, kernel version 2.6.32-358 or newer is required. To run without hole punching (at the cost of some efficiency and scalability), reconfigure -Kudu with --block_manager=file. Refer to the Kudu documentation for more -details. Raw error message follows. +Kudu to use the file block manager. Refer to the Kudu documentation for +more details. WARNING: the file block manager is not suitable for +production use and should be used only for small-scale evaluation and +development on systems where hole-punching is not available. It's +impossible to switch between block managers after data is written to the +server. Raw error message follows [NOTE] @@ -70,8 +74,18 @@ adding the `--block_manager=file` flag to the commands you use to start the mast and tablet servers. The file block manager does not scale as well as the log block manager. -WARNING: The file block manager is known to scale and perform poorly, and should -only be used for small-scale evaluation and development. +[WARNING] + +The file block manager is known to scale and perform poorly, and should +only be used for small-scale evaluation and development, and only on systems +where hole punching is unavailable. + +The file block manager uses one file per block. As multiple blocks are written +for each rowset, the number of blocks can be very high, especially for actively +written tablets. This can cause performance issues compared to the log block +manager even with a small amount of data and it's *impossible to switch between +block managers* without wiping and reinitializing the tablet servers. + [[disk_issues]] === Already present: FS layout already exists @@ -224,7 +238,7 @@ jitter, while `lpeers` lists dispersion. [NOTE] - + .Using `chrony` for time synchronization Some operating systems offer `chrony` as an alternative to `ntpd` for network time @@ -233,7 +247,7 @@ synchronization. Kudu has been tested most thoroughly using `ntpd` and use of In order to use `chrony` for synchronization, `chrony.conf` must be configured with the `rtcsync` option. - + NTP Configuration Best Practices http://git-wip-us.apache.org/repos/asf/kudu/blob/f1f192ca/src/kudu/fs/data_dirs.cc -- diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc index 49139ea..af0cbec 100644 --- a/src/kudu/fs/data_dirs.cc +++ b/src/kudu/fs/data_dirs.cc @@ -136,8 +136,12 @@ const char kHolePunchErrorMsg[] = "filesystem with hole punching support such as ext4 or xfs. On el6, " "kernel version 2.6.32-358 or newer is required. To run without hole " "punching (at the cost of some efficiency and scalability), reconfigure " -"Kudu with --block_manager=file. Refer to the Kudu documentation for more " -"details. Raw error message follows"; +"Kudu to use the file block manager. Refer to the Kudu
kudu git commit: Move constructor and assignment operator for Schema
Repository: kudu Updated Branches: refs/heads/master cf1b1f42c -> b848488d9 Move constructor and assignment operator for Schema This adds a move constructor and move assignment operator to Schema. I removed Schema::swap. It was only used in tests, and any potential use of it should be adequately fulfilled by one of moves, copies, or Schema::Reset. Change-Id: I6c827c84657875be4cf2bc565db03a686849d8e2 Reviewed-on: http://gerrit.cloudera.org:8080/11593 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/b848488d Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b848488d Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b848488d Branch: refs/heads/master Commit: b848488d9bd2efcfc63ddc073c3addece33793b6 Parents: cf1b1f4 Author: Will Berkeley Authored: Sun Sep 30 10:17:21 2018 -0700 Committer: Will Berkeley Committed: Fri Oct 5 18:38:54 2018 + -- src/kudu/common/id_mapping.h | 13 -- src/kudu/common/schema-test.cc | 80 - src/kudu/common/schema.cc | 49 ++- src/kudu/common/schema.h | 29 +++--- 4 files changed, 115 insertions(+), 56 deletions(-) -- http://git-wip-us.apache.org/repos/asf/kudu/blob/b848488d/src/kudu/common/id_mapping.h -- diff --git a/src/kudu/common/id_mapping.h b/src/kudu/common/id_mapping.h index a5d8289..f762b75 100644 --- a/src/kudu/common/id_mapping.h +++ b/src/kudu/common/id_mapping.h @@ -64,13 +64,6 @@ class IdMapping { clear(); } - explicit IdMapping(const IdMapping& other) - : mask_(other.mask_), -entries_(other.entries_) { - } - - ~IdMapping() {} - void clear() { ClearMap(_); } @@ -84,12 +77,6 @@ class IdMapping { other.entries_.swap(entries_); } - IdMapping& operator=(const IdMapping& other) { -mask_ = other.mask_; -entries_ = other.entries_; -return *this; - } - int operator[](int key) const { return get(key); } http://git-wip-us.apache.org/repos/asf/kudu/blob/b848488d/src/kudu/common/schema-test.cc -- diff --git a/src/kudu/common/schema-test.cc b/src/kudu/common/schema-test.cc index 503ebea..87ba59a 100644 --- a/src/kudu/common/schema-test.cc +++ b/src/kudu/common/schema-test.cc @@ -23,6 +23,7 @@ #include #include // IWYU pragma: keep #include +#include #include #include // IWYU pragma: keep @@ -102,6 +103,65 @@ TEST_F(TestSchema, TestSchema) { EXPECT_EQ("uint32 NULLABLE", schema.column(1).TypeToString()); } +TEST_F(TestSchema, TestCopyAndMove) { + auto check_schema = [](const Schema& schema) { +ASSERT_EQ(sizeof(Slice) + sizeof(uint32_t) + sizeof(int32_t), + schema.byte_size()); +ASSERT_EQ(3, schema.num_columns()); +ASSERT_EQ(0, schema.column_offset(0)); +ASSERT_EQ(sizeof(Slice), schema.column_offset(1)); + +EXPECT_EQ("Schema [\n" + "\tprimary key (key),\n" + "\tkey[string NOT NULL],\n" + "\tuint32val[uint32 NULLABLE],\n" + "\tint32val[int32 NOT NULL]\n" + "]", + schema.ToString()); +EXPECT_EQ("key[string NOT NULL]", schema.column(0).ToString()); +EXPECT_EQ("uint32 NULLABLE", schema.column(1).TypeToString()); + }; + + ColumnSchema col1("key", STRING); + ColumnSchema col2("uint32val", UINT32, true); + ColumnSchema col3("int32val", INT32); + + vector cols = { col1, col2, col3 }; + Schema schema(cols, 1); + check_schema(schema); + + // Check copy- and move-assignment. + Schema moved_schema; + { +Schema copied_schema = schema; +check_schema(copied_schema); +ASSERT_TRUE(copied_schema.Equals(schema)); + +// Move-assign to 'moved_to_schema' from 'copied_schema' and then let +// 'copied_schema' go out of scope to make sure none of 'moved_to_schema''s +// resources are incorrectly freed. +moved_schema = std::move(copied_schema); + +// 'copied_schema' is moved from so it should still be valid to call +// ToString(), though we can't expect any particular result. +NO_FATALS(copied_schema.ToString()); // NOLINT(*) + } + check_schema(moved_schema); + ASSERT_TRUE(moved_schema.Equals(schema)); + + // Check copy- and move-construction. + { +Schema copied_schema(schema); +check_schema(copied_schema); +ASSERT_TRUE(copied_schema.Equals(schema)); + +Schema moved_schema(std::move(copied_schema)); +NO_FATALS(copied_schema.ToString()); // NOLINT(*) +check_schema(moved_schema); +ASSERT_TRUE(moved_schema.Equals(schema)); + } +} + // Test basic functionality of Schema definition with decimal columns