kudu git commit: Extract connection retrying and HA support from HmsCatalog

2018-10-05 Thread danburkert
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

2018-10-05 Thread abukor
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

2018-10-05 Thread wdberkeley
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