You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/10/04 17:19:22 UTC

[kudu] branch master updated (fa6d5a2 -> 03e2ada)

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.


    from fa6d5a2  [mini_cluster] small fix on Mini{Chronyd,Hms}::Start()
     new d412939  [mini_chronyd] remove cmd socket directory on exit
     new 03e2ada  [test] use built-in NTP by default for ExternalMiniCluster

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/kudu/clock/test/mini_chronyd.cc            | 15 ++++++++++++---
 src/kudu/clock/test/mini_chronyd.h             |  3 +--
 src/kudu/mini-cluster/external_mini_cluster.cc |  2 +-
 src/kudu/util/test_util.cc                     | 23 ++++++++++++++++++-----
 src/kudu/util/test_util.h                      |  7 +++++++
 5 files changed, 39 insertions(+), 11 deletions(-)


[kudu] 01/02: [mini_chronyd] remove cmd socket directory on exit

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit d41293969d4cd908b7b030ac8a136fb457b50037
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Oct 3 16:32:22 2019 -0700

    [mini_chronyd] remove cmd socket directory on exit
    
    This patch adds a clean up for chronyd's command socket directory
    created by MiniChronyd.  Also, it removes unused cmd_socket_ member
    of the MiniChronyd class.
    
    Change-Id: I2139857f6312fab0839cf23a6c66b3448c632791
    Reviewed-on: http://gerrit.cloudera.org:8080/14365
    Tested-by: Alexey Serbin <as...@cloudera.com>
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 src/kudu/clock/test/mini_chronyd.cc | 15 ++++++++++++---
 src/kudu/clock/test/mini_chronyd.h  |  3 +--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/kudu/clock/test/mini_chronyd.cc b/src/kudu/clock/test/mini_chronyd.cc
index 7f1d09f..1d86a40 100644
--- a/src/kudu/clock/test/mini_chronyd.cc
+++ b/src/kudu/clock/test/mini_chronyd.cc
@@ -156,7 +156,14 @@ MiniChronyd::MiniChronyd(MiniChronydOptions options)
 
 MiniChronyd::~MiniChronyd() {
   if (process_) {
-    WARN_NOT_OK(Stop(), "unable to stop MiniChronyd");
+    WARN_NOT_OK(Stop(), "unable to stop chronyd");
+  }
+  if (!cmd_socket_dir_.empty()) {
+    // Remove the directory created for the command Unix domain socket. The
+    // directory should be empty if chronyd exited normally: it cleans up after
+    // itself on a clean exit.
+    WARN_NOT_OK(Env::Default()->DeleteDir(cmd_socket_dir_),
+                "unable to remove chronyd command socket directory");
   }
 }
 
@@ -390,14 +397,16 @@ $0
     // TODO(aserbin): use some synthetic mount point instead?
     string dir;
     RETURN_NOT_OK(Env::Default()->GetTestDirectory(&dir));
-    dir += Substitute("/$0.$1", Env::Default()->NowMicros(), getpid());
+    dir = JoinPathSegments(dir, Substitute("$0.$1", Env::Default()->NowMicros(),
+                                           getpid()));
     const auto s = Env::Default()->CreateDir(dir);
-    if (!s.IsAlreadyPresent() && !s.ok()) {
+    if (!s.ok() && !s.IsAlreadyPresent()) {
       return s;
     }
     RETURN_NOT_OK(CorrectOwnership(dir));
     options_.bindcmdaddress = Substitute("$0/chronyd.$1.sock",
                                          dir, options_.index);
+    cmd_socket_dir_ = std::move(dir);
   }
   string username;
   RETURN_NOT_OK(GetLoggedInUser(&username));
diff --git a/src/kudu/clock/test/mini_chronyd.h b/src/kudu/clock/test/mini_chronyd.h
index b7bd45e..c888fe9 100644
--- a/src/kudu/clock/test/mini_chronyd.h
+++ b/src/kudu/clock/test/mini_chronyd.h
@@ -29,7 +29,6 @@
 namespace kudu {
 
 class HostPort;
-class RWFile;
 class Subprocess;
 
 namespace clock {
@@ -249,7 +248,7 @@ class MiniChronyd {
                       std::string* out_stderr = nullptr) const WARN_UNUSED_RESULT;
 
   MiniChronydOptions options_;
-  std::unique_ptr<RWFile> cmd_socket_;
+  std::string cmd_socket_dir_;
   std::unique_ptr<Subprocess> process_;
 };
 


[kudu] 02/02: [test] use built-in NTP by default for ExternalMiniCluster

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 03e2ada694290cafce0bea6ebbf092709aa64a2a
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Oct 3 12:43:38 2019 -0700

    [test] use built-in NTP by default for ExternalMiniCluster
    
    This patch switches all ExternalMiniCluster-based tests to use
    the built-in NTP client as the default source for HybridTime timestamps.
    
    A new environment variable is introduced to override the default when
    running tests: KUDU_USE_SYSTEM_NTP.  Set it to true if it's desired
    to run the ExternalMiniCluster-based scenarios using the machine's local
    clock as the source for HybridTime (the clock should be synchronized by
    the kernel NTP discipline).
    
    Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
    Reviewed-on: http://gerrit.cloudera.org:8080/14362
    Tested-by: Alexey Serbin <as...@cloudera.com>
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 src/kudu/mini-cluster/external_mini_cluster.cc |  2 +-
 src/kudu/util/test_util.cc                     | 23 ++++++++++++++++++-----
 src/kudu/util/test_util.h                      |  7 +++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index 23814be..b2f01bc 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -116,7 +116,7 @@ ExternalMiniClusterOptions::ExternalMiniClusterOptions()
       logtostderr(true),
       start_process_timeout(MonoDelta::FromSeconds(70)),
       rpc_negotiation_timeout(MonoDelta::FromSeconds(3)),
-      num_ntp_servers(0) {
+      num_ntp_servers(UseSystemNtp() ? 0 : 1) {
 }
 
 ExternalMiniCluster::ExternalMiniCluster()
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index c514d0e..c41d1d1 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -73,7 +73,8 @@ using strings::Substitute;
 namespace kudu {
 
 const char* kInvalidPath = "/dev/invalid-path-for-kudu-tests";
-static const char* const kSlowTestsEnvVariable = "KUDU_ALLOW_SLOW_TESTS";
+static const char* const kSlowTestsEnvVar = "KUDU_ALLOW_SLOW_TESTS";
+static const char* const kUseSystemNtpEnvVar = "KUDU_USE_SYSTEM_NTP";
 
 static const uint64_t kTestBeganAtMicros = Env::Default()->NowMicros();
 
@@ -169,8 +170,10 @@ void KuduTest::OverrideKrb5Environment() {
 // Test utility functions
 ///////////////////////////////////////////////////
 
-bool AllowSlowTests() {
-  char *e = getenv(kSlowTestsEnvVariable);
+namespace {
+// Get the value of an environment variable that has boolean semantics.
+bool GetBooleanEnvironmentVariable(const char* env_var_name) {
+  const char* const e = getenv(env_var_name);
   if ((e == nullptr) ||
       (strlen(e) == 0) ||
       (strcasecmp(e, "false") == 0) ||
@@ -183,8 +186,18 @@ bool AllowSlowTests() {
       (strcasecmp(e, "yes") == 0)) {
     return true;
   }
-  LOG(FATAL) << "Unrecognized value for " << kSlowTestsEnvVariable << ": " << e;
-  return false;
+  LOG(FATAL) << Substitute("$0: invalid value for environment variable $0",
+                           e, env_var_name);
+  return false;  // unreachable
+}
+} // anonymous namespace
+
+bool AllowSlowTests() {
+  return GetBooleanEnvironmentVariable(kSlowTestsEnvVar);
+}
+
+bool UseSystemNtp() {
+  return GetBooleanEnvironmentVariable(kUseSystemNtpEnvVar);
 }
 
 void OverrideFlagForSlowTests(const std::string& flag_name,
diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h
index d320e3e..7e9764d 100644
--- a/src/kudu/util/test_util.h
+++ b/src/kudu/util/test_util.h
@@ -87,6 +87,13 @@ class KuduTest : public ::testing::Test {
 // Returns true if slow tests are runtime-enabled.
 bool AllowSlowTests();
 
+// Returns true if tests should rely on the system clock synchronized by the
+// kernel NTP discipline. By default, test clusters should run their own
+// test NTP server and configure Kudu masters and tablet servers to use the
+// built-in NTP client as the clock source for HybridTime, where the built-in
+// NTP clients are pointed to the test cluster's dedicated NTP server.
+bool UseSystemNtp();
+
 // Override the given gflag to the new value, only in the case that
 // slow tests are enabled and the user hasn't otherwise overridden
 // it on the command line.