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 2020/03/11 16:40:06 UTC
[kudu] branch master updated (0b858e6 -> a9afbce)
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 0b858e6 [build] Fix relocate_binaries_for_mini_cluster.py on Python 3
new d74ad32 KUDU-3050: recover from corrupt kerberos ccache
new a9afbce [ksck] report on misconfiguration for flag categories
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/integration-tests/security-itest.cc | 39 +++
src/kudu/rebalance/cluster_status.cc | 6 +-
src/kudu/rebalance/cluster_status.h | 6 +-
src/kudu/rpc/client_negotiation.cc | 7 +
src/kudu/security/init.cc | 117 ++++-----
src/kudu/security/kinit_context.h | 85 +++++++
src/kudu/security/test/mini_kdc.cc | 11 +-
src/kudu/security/test/mini_kdc.h | 3 +
src/kudu/tools/ksck-test.cc | 47 ++--
src/kudu/tools/ksck.cc | 348 ++++++++++++++++++++++-----
src/kudu/tools/ksck.h | 144 ++++++++---
src/kudu/tools/ksck_remote.cc | 71 +++---
src/kudu/tools/ksck_remote.h | 4 +-
src/kudu/tools/ksck_results.cc | 100 ++++++--
src/kudu/tools/ksck_results.h | 29 ++-
src/kudu/tools/tool_action_cluster.cc | 1 +
16 files changed, 782 insertions(+), 236 deletions(-)
create mode 100644 src/kudu/security/kinit_context.h
[kudu] 01/02: KUDU-3050: recover from corrupt kerberos ccache
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 d74ad32df7e41f6c9b03edb8b7be27706b507c2c
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Thu Feb 6 18:05:35 2020 -0800
KUDU-3050: recover from corrupt kerberos ccache
This handles two failure modes:
* krb5_cc_start_seq_get() can fail if the kerberos credential cache gets
corrupted on disk, e.g. is truncated.
* the renewal can fail to find a credential in the credential cache,
either if it is missing or the renewal thread hits an error while
reading through credentials.
Also add some additional logging and limit the max backoff time
to make it easier to debug other kinds of renewal errors.
The test triggers a pre-existing memory leak bug in some older
Kerberos libraries. Added a suppression for leak sanitizer
to ClientNegotiation::CheckGSSAPI() to suppress it.
Test:
Add a test that exercises the recovery logic after truncating
the credential cache. The test failed before this change.
Change-Id: I2d6e06c3ea65708896a6bf0134cc84838b3f1b58
Reviewed-on: http://gerrit.cloudera.org:8080/15394
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/integration-tests/security-itest.cc | 39 +++++++++
src/kudu/rpc/client_negotiation.cc | 7 ++
src/kudu/security/init.cc | 117 ++++++++++++++-------------
src/kudu/security/kinit_context.h | 85 +++++++++++++++++++
src/kudu/security/test/mini_kdc.cc | 11 ++-
src/kudu/security/test/mini_kdc.h | 3 +
6 files changed, 200 insertions(+), 62 deletions(-)
diff --git a/src/kudu/integration-tests/security-itest.cc b/src/kudu/integration-tests/security-itest.cc
index 1003461..905adee 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -18,6 +18,8 @@
#include <sys/stat.h>
#include <cstdio>
+#include <cstdlib>
+#include <initializer_list>
#include <memory>
#include <ostream>
#include <string>
@@ -46,6 +48,7 @@
#include "kudu/mini-cluster/external_mini_cluster.h"
#include "kudu/rpc/messenger.h"
#include "kudu/rpc/rpc_controller.h"
+#include "kudu/security/kinit_context.h"
#include "kudu/security/test/mini_kdc.h"
#include "kudu/security/token.pb.h"
#include "kudu/server/server_base.pb.h"
@@ -397,6 +400,42 @@ TEST_F(SecurityITest, TestWorldReadablePrivateKey) {
credentials_name));
}
+// Test that our Kinit implementation can handle corrupted credential caches.
+TEST_F(SecurityITest, TestCorruptKerberosCC) {
+ ASSERT_OK(StartCluster());
+ string admin_keytab = cluster_->kdc()->GetKeytabPathForPrincipal("test-admin");
+ ASSERT_OK(cluster_->kdc()->CreateKeytabForExistingPrincipal("test-admin"));
+
+ security::KinitContext kinit_ctx;
+ ASSERT_OK(kinit_ctx.Kinit(admin_keytab, "test-admin"));
+
+ // Truncate at different lengths to exercise different failure modes, e.g. failed to
+ // read header, some credentials missing.
+ for (int trunc_len : {10, 75, 500}) {
+ // Truncate the credential cache so that it no longer contains a valid ticket for
+ // "test-admin".
+ const char* cc_path = getenv("KRB5CCNAME");
+ SCOPED_TRACE(Substitute("Truncating ccache at '$0' to $1", cc_path, trunc_len));
+ {
+ RWFileOptions opts;
+ opts.mode = Env::MUST_EXIST;
+ unique_ptr<RWFile> cc_file;
+ ASSERT_OK(env_->NewRWFile(opts, cc_path, &cc_file));
+ ASSERT_OK(cc_file->Truncate(trunc_len));
+ }
+
+ // With corrupt cache, we shouldn't be able to open connection.
+ Status s = TrySetFlagOnTS();
+ EXPECT_FALSE(s.ok());
+ ASSERT_STR_CONTAINS(s.ToString(), "server requires authentication, but client does "
+ "not have Kerberos credentials available");
+
+ // Renewal should fix the corrupted credential cache and allow secure connections.
+ ASSERT_OK(kinit_ctx.DoRenewal());
+ ASSERT_OK(TrySetFlagOnTS());
+ }
+}
+
Status AssignIPToClient(bool external) {
// If the test does not require an external IP address
// assign loopback IP to FLAGS_local_ip_for_outbound_sockets.
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index b43f55d..4129193 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -47,6 +47,7 @@
#include "kudu/security/gssapi.h"
#include "kudu/security/tls_context.h"
#include "kudu/security/tls_handshake.h"
+#include "kudu/util/debug/leakcheck_disabler.h"
#include "kudu/util/faststring.h"
#include "kudu/util/net/sockaddr.h"
#include "kudu/util/net/socket.h"
@@ -793,6 +794,12 @@ int ClientNegotiation::SecretCb(sasl_conn_t* conn, int id, sasl_secret_t** psecr
}
Status ClientNegotiation::CheckGSSAPI() {
+ // Disable leak checking in this function to work around memory leak in libgssapi_krb5
+ // when opening a corrupt credential cache fails:
+ // https://krbdev.mit.edu/rt/Ticket/Display.html?id=8437.
+ // Fixed in MIT Kerberos 1.13.7, 1.14.4 and 1.15.
+ debug::ScopedLeakCheckDisabler disable_leak_checks;
+
OM_uint32 major, minor;
gss_cred_id_t cred = GSS_C_NO_CREDENTIAL;
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index d0b2226..e740fcb 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -30,6 +30,7 @@
#include <random>
#include <string>
#include <type_traits>
+#include <utility>
#include <boost/optional/optional.hpp>
#include <gflags/gflags.h>
@@ -40,6 +41,7 @@
#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/gutil/strings/util.h"
+#include "kudu/security/kinit_context.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/monotime.h"
#include "kudu/util/net/net_util.h"
@@ -82,67 +84,19 @@ using strings::Substitute;
namespace kudu {
namespace security {
-namespace {
+// Global instance of the context used by the kinit/reacquire thread.
+KinitContext* g_kinit_ctx;
-class KinitContext;
+namespace {
// Global context for usage of the Krb5 library.
krb5_context g_krb5_ctx;
-// Global instance of the context used by the kinit/reacquire thread.
-KinitContext* g_kinit_ctx;
-
// This lock is used to avoid a race while reacquiring the kerberos ticket.
// The race can occur between the time we reinitialize the cache and the
// time when we actually store the new credentials back in the cache.
RWMutex* g_kerberos_reinit_lock;
-class KinitContext {
- public:
- KinitContext();
-
- // Equivalent implementation of 'kinit -kt <keytab path> <principal>'.
- //
- // This logs in from the given keytab as the given principal, returning
- // RuntimeError if any part of this process fails.
- //
- // If the log-in is successful, then the default ticket cache is overwritten
- // with the credentials of the newly logged-in principal.
- Status Kinit(const string& keytab_path, const string& principal);
-
- // Acquires a new Ticket Granting Ticket (TGT).
- //
- // Renews the existing ticket if possible, or acquires a new Ticket Granting
- // Ticket (TGT).
- Status DoRenewal();
-
- // Calculates the next sleep interval based on the 'ticket_end_timestamp_' and
- // adds some jitter so that all the nodes do not hit the KDC at the same time.
- //
- // If 'num_retries' > 0, it calls GetBackedOffRenewInterval() to return a backed
- // off interval.
- int32_t GetNextRenewInterval(uint32_t num_retries);
-
- // Returns a value based on 'time_remaining' that increases exponentially with
- // 'num_retries', with a random jitter of +/- 0%-50% of that value.
- int32_t GetBackedOffRenewInterval(int32_t time_remaining, uint32_t num_retries);
-
- const string& principal_str() const { return principal_str_; }
- const string& username_str() const { return username_str_; }
-
- private:
- krb5_principal principal_;
- krb5_keytab keytab_;
- krb5_ccache ccache_;
- krb5_get_init_creds_opt* opts_;
-
- // The stringified principal and username that we are logged in as.
- string principal_str_, username_str_;
-
- // This is the time that the current TGT in use expires.
- int32_t ticket_end_timestamp_;
-};
-
Status Krb5CallToStatus(krb5_context ctx, krb5_error_code code) {
if (code == 0) return Status::OK();
@@ -171,11 +125,11 @@ void InitKrb5Ctx() {
char* unused_realm;
CHECK_EQ(krb5_get_default_realm(g_krb5_ctx, &unused_realm), 0);
krb5_free_default_realm(g_krb5_ctx, unused_realm);
+
+ g_kerberos_reinit_lock = new RWMutex(RWMutex::Priority::PREFER_WRITING);
});
}
-KinitContext::KinitContext() {}
-
// Port of the data_eq() implementation from krb5/k5-int.h
inline int data_eq(krb5_data d1, krb5_data d2) {
return (d1.length == d2.length && !memcmp(d1.data, d2.data, d1.length));
@@ -202,7 +156,13 @@ void RenewThread() {
uint32_t failure_retries = 0;
while (true) {
// This thread is run immediately after the first Kinit, so sleep first.
- SleepFor(MonoDelta::FromSeconds(g_kinit_ctx->GetNextRenewInterval(failure_retries)));
+ int64_t renew_interval_s = g_kinit_ctx->GetNextRenewInterval(failure_retries);
+ if (failure_retries > 0) {
+ // Log in the abnormal case where something failed.
+ LOG(INFO) << Substitute("Renew thread sleeping after $0 failures for $1s",
+ failure_retries, renew_interval_s);
+ }
+ SleepFor(MonoDelta::FromSeconds(renew_interval_s));
Status s = g_kinit_ctx->DoRenewal();
WARN_NOT_OK(s, "Kerberos reacquire error: ");
@@ -213,6 +173,17 @@ void RenewThread() {
}
}
}
+} // anonymous namespace
+
+KinitContext::KinitContext() {}
+
+KinitContext::~KinitContext() {
+ // Free memory associated with these objects.
+ if (principal_ != nullptr) krb5_free_principal(g_krb5_ctx, principal_);
+ if (keytab_ != nullptr) krb5_kt_close(g_krb5_ctx, keytab_);
+ if (ccache_ != nullptr) krb5_cc_close(g_krb5_ctx, ccache_);
+ if (opts_ != nullptr) krb5_get_init_creds_opt_free(g_krb5_ctx, opts_);
+}
int32_t KinitContext::GetNextRenewInterval(uint32_t num_retries) {
int32_t time_remaining = ticket_end_timestamp_ - time(nullptr);
@@ -242,7 +213,8 @@ int32_t KinitContext::GetNextRenewInterval(uint32_t num_retries) {
int32_t KinitContext::GetBackedOffRenewInterval(int32_t time_remaining, uint32_t num_retries) {
// The minimum sleep interval after a failure will be 60 seconds.
int32_t next_interval = std::max(time_remaining, 60);
- int32_t base_time = std::min(next_interval * (1 << num_retries), INT32_MAX);
+ // Don't back off more than 30 minutes.
+ int32_t base_time = std::min(next_interval * (1 << num_retries), 1800);
random_device rd;
mt19937 generator(rd());
uniform_real_distribution<> dist(0.5, 1.5);
@@ -250,11 +222,31 @@ int32_t KinitContext::GetBackedOffRenewInterval(int32_t time_remaining, uint32_t
}
Status KinitContext::DoRenewal() {
+ bool found_in_cache;
+ RETURN_NOT_OK(DoRenewalInternal(&found_in_cache));
+ if (!found_in_cache) {
+ // Cache appears to be corrupted somehow - redo the kinit and retry.
+ RETURN_NOT_OK_PREPEND(KinitInternal(), "redoing kinit after error");
+ RETURN_NOT_OK(DoRenewalInternal(&found_in_cache));
+ if (!found_in_cache) {
+ return Status::RuntimeError("Could not find TGT in cache after kinit");
+ }
+ }
+ return Status::OK();
+}
+Status KinitContext::DoRenewalInternal(bool* found_in_cache) {
+ *found_in_cache = false;
krb5_cc_cursor cursor;
// Setup a cursor to iterate through the credential cache.
- KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_start_seq_get(g_krb5_ctx, ccache_, &cursor),
- "Failed to peek into ccache");
+ Status status =
+ Krb5CallToStatus(g_krb5_ctx, krb5_cc_start_seq_get(g_krb5_ctx, ccache_, &cursor));
+ if (!status.ok()) {
+ LOG(WARNING) << Substitute("Error while opening credential cache '$0' of type $1: $2",
+ krb5_cc_get_name(g_krb5_ctx, ccache_), krb5_cc_get_type(g_krb5_ctx, ccache_),
+ status.ToString());
+ return Status::OK();
+ }
SCOPED_CLEANUP({
krb5_cc_end_seq_get(g_krb5_ctx, ccache_, &cursor); });
@@ -276,6 +268,7 @@ Status KinitContext::DoRenewal() {
data_eq(creds.server->realm, principal_->realm) == 0) {
continue;
}
+ *found_in_cache = true;
krb5_creds new_creds;
memset(&new_creds, 0, sizeof(krb5_creds));
@@ -304,6 +297,11 @@ Status KinitContext::DoRenewal() {
ticket_end_timestamp_ = new_creds.times.endtime;
break;
}
+ if (!*found_in_cache) {
+ LOG(WARNING) << Substitute(
+ "Could not find kerberos principal in credential cache '$0' of type $1",
+ krb5_cc_get_name(g_krb5_ctx, ccache_), krb5_cc_get_type(g_krb5_ctx, ccache_));
+ }
return Status::OK();
}
@@ -322,7 +320,10 @@ Status KinitContext::Kinit(const string& keytab_path, const string& principal) {
KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_alloc(g_krb5_ctx, &opts_),
"unable to allocate get_init_creds_opt struct");
+ return KinitInternal();
+}
+Status KinitContext::KinitInternal() {
#if defined(HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_CCACHE)
KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_set_out_ccache(g_krb5_ctx, opts_, ccache_),
"unable to set init_creds options");
@@ -362,6 +363,7 @@ Status KinitContext::Kinit(const string& keytab_path, const string& principal) {
return Status::OK();
}
+namespace {
// 'in_principal' is the user specified principal to use with Kerberos. It may have a token
// in the string of the form '_HOST', which if present, needs to be replaced with the FQDN of the
// current host.
@@ -491,7 +493,6 @@ Status InitKerberosForServer(const std::string& raw_principal, const std::string
RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(
keytab_file, configured_principal), "unable to kinit");
- g_kerberos_reinit_lock = new RWMutex(RWMutex::Priority::PREFER_WRITING);
scoped_refptr<Thread> reacquire_thread;
// Start the reacquire thread.
RETURN_NOT_OK(Thread::Create("kerberos", "reacquire thread", &RenewThread, &reacquire_thread));
diff --git a/src/kudu/security/kinit_context.h b/src/kudu/security/kinit_context.h
new file mode 100644
index 0000000..a6c2b41
--- /dev/null
+++ b/src/kudu/security/kinit_context.h
@@ -0,0 +1,85 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <cstdint>
+#include <string>
+
+#include <krb5/krb5.h>
+
+#include "kudu/util/status.h"
+
+namespace kudu {
+namespace security {
+
+class KinitContext {
+ public:
+ KinitContext();
+ ~KinitContext();
+
+ // Equivalent implementation of 'kinit -kt <keytab path> <principal>'.
+ //
+ // This logs in from the given keytab as the given principal, returning
+ // RuntimeError if any part of this process fails.
+ //
+ // If the log-in is successful, then the default ticket cache is overwritten
+ // with the credentials of the newly logged-in principal.
+ Status Kinit(const std::string& keytab_path, const std::string& principal);
+
+ // Acquires a new Ticket Granting Ticket (TGT).
+ //
+ // Renews the existing ticket if possible, or acquires a new Ticket Granting
+ // Ticket (TGT).
+ Status DoRenewal();
+
+ // Calculates the next sleep interval based on the 'ticket_end_timestamp_' and
+ // adds some jitter so that all the nodes do not hit the KDC at the same time.
+ //
+ // If 'num_retries' > 0, it calls GetBackedOffRenewInterval() to return a backed
+ // off interval.
+ int32_t GetNextRenewInterval(uint32_t num_retries);
+
+ // Returns a value based on 'time_remaining' that increases exponentially with
+ // 'num_retries', with a random jitter of +/- 0%-50% of that value.
+ static int32_t GetBackedOffRenewInterval(int32_t time_remaining, uint32_t num_retries);
+
+ const std::string& principal_str() const { return principal_str_; }
+ const std::string& username_str() const { return username_str_; }
+
+ private:
+ Status KinitInternal();
+
+ // Helper for DoRenewal() that tries to do a renewal. On success, returns OK and sets
+ // *found_in_cache = true. If there is an error doing the renewal itself, returns an
+ // error. If the TGT to be renewed was not found in the cache, return OK and set
+ // *found_in_cache = false.
+ Status DoRenewalInternal(bool* found_in_cache);
+
+ krb5_principal principal_ = nullptr;
+ krb5_keytab keytab_ = nullptr;
+ krb5_ccache ccache_ = nullptr;
+ krb5_get_init_creds_opt* opts_ = nullptr;
+
+ // The stringified principal and username that we are logged in as.
+ std::string principal_str_, username_str_;
+
+ // This is the time that the current TGT in use expires.
+ int32_t ticket_end_timestamp_;
+};
+
+} // namespace security
+} // namespace kudu
diff --git a/src/kudu/security/test/mini_kdc.cc b/src/kudu/security/test/mini_kdc.cc
index 1662770..e3d9604 100644
--- a/src/kudu/security/test/mini_kdc.cc
+++ b/src/kudu/security/test/mini_kdc.cc
@@ -287,10 +287,7 @@ Status MiniKdc::RandomizePrincipalKey(const string& spn) {
Status MiniKdc::CreateKeytabForExistingPrincipal(const string& spn) {
SCOPED_LOG_SLOW_EXECUTION(WARNING, 100, Substitute("creating keytab for $0", spn));
- string kt_path = spn;
- StripString(&kt_path, "/", '_');
- kt_path = JoinPathSegments(options_.data_root, kt_path) + ".keytab";
-
+ string kt_path = GetKeytabPathForPrincipal(spn);
string kadmin;
RETURN_NOT_OK(GetBinaryPath("kadmin.local", &kadmin));
RETURN_NOT_OK(Subprocess::Call(MakeArgv({
@@ -298,6 +295,12 @@ Status MiniKdc::CreateKeytabForExistingPrincipal(const string& spn) {
return Status::OK();
}
+string MiniKdc::GetKeytabPathForPrincipal(const string& spn) const {
+ string kt_path = spn;
+ StripString(&kt_path, "/", '_');
+ return JoinPathSegments(options_.data_root, kt_path) + ".keytab";
+}
+
Status MiniKdc::Kinit(const string& username) {
SCOPED_LOG_SLOW_EXECUTION(WARNING, 100, Substitute("kinit for $0", username));
string kinit;
diff --git a/src/kudu/security/test/mini_kdc.h b/src/kudu/security/test/mini_kdc.h
index 94990a2..10dc669 100644
--- a/src/kudu/security/test/mini_kdc.h
+++ b/src/kudu/security/test/mini_kdc.h
@@ -100,6 +100,9 @@ class MiniKdc {
// 'spn' is the desired service principal name (e.g. "kudu/foo.example.com").
Status CreateKeytabForExistingPrincipal(const std::string& spn);
+ // Returns the path where CreateKeytabForExistingPrincipal creates the keytab.
+ std::string GetKeytabPathForPrincipal(const std::string& spn) const;
+
// Kinit a user to the mini KDC.
Status Kinit(const std::string& username) WARN_UNUSED_RESULT;
[kudu] 02/02: [ksck] report on misconfiguration for flag categories
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 a9afbce14454f9b03b9cc5a699a09f4f9935da58
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Feb 25 18:48:05 2020 -0800
[ksck] report on misconfiguration for flag categories
This patch adds functionality to report on misconfiguration of flags
in pre-defined flag categories. The 'misconfiguration' means different
values for same flags in each group of masters/tablet servers in a
cluster (i.e. intra-group differences), and also differences between
masters' and tablet servers' flags (i.e. cross-group differences).
With this patch, two categories are defined and ready for use:
* 'time_source' includes the following flags
** --builtin_ntp_servers
** --time_source
* 'unusual' includes all experimental, hidden, and unsafe flags
The newly introduced functionality is enabled by default for the
'time_source' flags category. The list of group categories to check
can be customized by adding
--flags_categories_to_check=<comma-separated-list-of-flags-categories>
to the 'kudu cluster ksck' invocation. Use an empty string to specify
an empty list of flags categories to check.
For testing, I ran a small Kudu cluster and verified that ksck reports
on misconfiguration as expected:
* if tablet servers are run with different flags
* if masters are run with flag settings different from those used for
tablet servers
This patch also contains assorted re-factoring in related areas
of code. I'm planning to add automated tests to cover the newly
introduced functionality in a separate patch.
Change-Id: I2afbfac9327f85b212ecf8d8f43a2139f90db6bb
Reviewed-on: http://gerrit.cloudera.org:8080/15298
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
src/kudu/rebalance/cluster_status.cc | 6 +-
src/kudu/rebalance/cluster_status.h | 6 +-
src/kudu/tools/ksck-test.cc | 47 +++--
src/kudu/tools/ksck.cc | 348 ++++++++++++++++++++++++++++------
src/kudu/tools/ksck.h | 144 ++++++++++----
src/kudu/tools/ksck_remote.cc | 71 ++++---
src/kudu/tools/ksck_remote.h | 4 +-
src/kudu/tools/ksck_results.cc | 100 ++++++++--
src/kudu/tools/ksck_results.h | 29 ++-
src/kudu/tools/tool_action_cluster.cc | 1 +
10 files changed, 582 insertions(+), 174 deletions(-)
diff --git a/src/kudu/rebalance/cluster_status.cc b/src/kudu/rebalance/cluster_status.cc
index 685b059..38b4da6 100644
--- a/src/kudu/rebalance/cluster_status.cc
+++ b/src/kudu/rebalance/cluster_status.cc
@@ -25,7 +25,7 @@
namespace kudu {
namespace cluster_summary {
-const char* const HealthCheckResultToString(HealthCheckResult cr) {
+const char* HealthCheckResultToString(HealthCheckResult cr) {
switch (cr) {
case HealthCheckResult::HEALTHY:
return "HEALTHY";
@@ -43,7 +43,7 @@ const char* const HealthCheckResultToString(HealthCheckResult cr) {
}
// Return a string representation of 'sh'.
-const char* const ServerHealthToString(ServerHealth sh) {
+const char* ServerHealthToString(ServerHealth sh) {
switch (sh) {
case ServerHealth::HEALTHY:
return "HEALTHY";
@@ -59,7 +59,7 @@ const char* const ServerHealthToString(ServerHealth sh) {
}
// Return a string representation of 'type'.
-const char* const ServerTypeToString(ServerType type) {
+const char* ServerTypeToString(ServerType type) {
switch (type) {
case ServerType::MASTER:
return "Master";
diff --git a/src/kudu/rebalance/cluster_status.h b/src/kudu/rebalance/cluster_status.h
index 8d6ea8c..cc5dcbf 100644
--- a/src/kudu/rebalance/cluster_status.h
+++ b/src/kudu/rebalance/cluster_status.h
@@ -56,7 +56,7 @@ enum class HealthCheckResult {
CONSENSUS_MISMATCH,
};
-const char* const HealthCheckResultToString(HealthCheckResult cr);
+const char* HealthCheckResultToString(HealthCheckResult cr);
// Possible types of consensus configs.
enum class ConsensusConfigType {
@@ -127,7 +127,7 @@ enum class ServerHealth {
};
// Return a string representation of 'sh'.
-const char* const ServerHealthToString(ServerHealth sh);
+const char* ServerHealthToString(ServerHealth sh);
// Quiescing-related info.
struct QuiescingInfo {
@@ -193,7 +193,7 @@ enum class ServerType {
};
// Return a string representation of 'type'.
-const char* const ServerTypeToString(ServerType type);
+const char* ServerTypeToString(ServerType type);
// A summary of the state of a tablet replica.
struct ReplicaSummary {
diff --git a/src/kudu/tools/ksck-test.cc b/src/kudu/tools/ksck-test.cc
index 13b669d..45cfa3f 100644
--- a/src/kudu/tools/ksck-test.cc
+++ b/src/kudu/tools/ksck-test.cc
@@ -19,6 +19,7 @@
#include <algorithm>
#include <atomic>
+#include <cstddef>
#include <cstdint>
#include <initializer_list>
#include <map>
@@ -92,7 +93,9 @@ class MockKsckMaster : public KsckMaster {
uuid_ = uuid;
version_ = "mock-version";
if (is_get_flags_available_) {
- unusual_flags_.emplace();
+ for (size_t cat = FlagsCategory::MIN; cat < FlagsCategory::ARRAY_SIZE; ++cat) {
+ flags_by_category_[cat].flags.emplace();
+ }
}
}
@@ -113,13 +116,16 @@ class MockKsckMaster : public KsckMaster {
return fetch_cstate_status_;
}
- Status FetchUnusualFlags() override {
- if (is_get_flags_available_) {
- unusual_flags_state_ = KsckFetchState::FETCHED;
- return Status::OK();
+ Status FetchFlags(const std::vector<FlagsCategory>& categories) override {
+ for (const auto cat : categories) {
+ if (is_get_flags_available_) {
+ flags_by_category_[cat].state = KsckFetchState::FETCHED;
+ } else {
+ flags_by_category_[cat].state = KsckFetchState::FETCH_FAILED;
+ }
}
- unusual_flags_state_ = KsckFetchState::FETCH_FAILED;
- return Status::RemoteError("GetFlags not available");
+ return is_get_flags_available_
+ ? Status::OK() : Status::RemoteError("GetFlags not available");
}
// Public because the unit tests mutate these variables directly.
@@ -127,7 +133,7 @@ class MockKsckMaster : public KsckMaster {
Status fetch_cstate_status_;
using KsckMaster::uuid_;
using KsckMaster::cstate_;
- using KsckMaster::unusual_flags_;
+ using KsckMaster::flags_by_category_;
using KsckMaster::version_;
private:
const bool is_get_flags_available_;
@@ -143,7 +149,9 @@ class MockKsckTabletServer : public KsckTabletServer {
is_get_flags_available_(is_get_flags_available) {
version_ = "mock-version";
if (is_get_flags_available_) {
- unusual_flags_.emplace();
+ for (size_t cat = FlagsCategory::MIN; cat < FlagsCategory::ARRAY_SIZE; ++cat) {
+ flags_by_category_[cat].flags.emplace();
+ }
}
}
@@ -163,13 +171,16 @@ class MockKsckTabletServer : public KsckTabletServer {
return Status::OK();
}
- Status FetchUnusualFlags() override {
- if (is_get_flags_available_) {
- unusual_flags_state_ = KsckFetchState::FETCHED;
- return Status::OK();
+ Status FetchFlags(const std::vector<FlagsCategory>& categories) override {
+ for (const auto cat : categories) {
+ if (is_get_flags_available_) {
+ flags_by_category_[cat].state = KsckFetchState::FETCHED;
+ } else {
+ flags_by_category_[cat].state = KsckFetchState::FETCH_FAILED;
+ }
}
- unusual_flags_state_ = KsckFetchState::FETCH_FAILED;
- return Status::RemoteError("GetFlags not available");
+ return is_get_flags_available_
+ ? Status::OK() : Status::RemoteError("GetFlags not available");
}
void FetchCurrentTimestampAsync() override {}
@@ -203,7 +214,7 @@ class MockKsckTabletServer : public KsckTabletServer {
// The fake progress amount for this mock server, used to mock checksum
// progress for this server.
int64_t checksum_progress_ = 10;
- using KsckTabletServer::unusual_flags_;
+ using KsckTabletServer::flags_by_category_;
using KsckTabletServer::location_;
using KsckTabletServer::version_;
@@ -1100,7 +1111,7 @@ TEST_F(KsckTest, TestMasterFlagCheck) {
}
shared_ptr<MockKsckMaster> master =
std::static_pointer_cast<MockKsckMaster>(cluster_->masters_.at(i));
- master->unusual_flags_ = std::move(flags);
+ master->flags_by_category_[FlagsCategory::UNUSUAL].flags = std::move(flags);
}
ASSERT_OK(ksck_->CheckMasterHealth());
ASSERT_OK(ksck_->CheckMasterUnusualFlags());
@@ -1233,7 +1244,7 @@ TEST_F(KsckTest, TestTserverFlagCheck) {
}
shared_ptr<MockKsckTabletServer> ts =
std::static_pointer_cast<MockKsckTabletServer>(entry.second);
- ts->unusual_flags_ = std::move(flags);
+ ts->flags_by_category_[FlagsCategory::UNUSUAL].flags = std::move(flags);
i++;
}
ASSERT_OK(ksck_->FetchInfoFromTabletServers());
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index 0582de9..310007d 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -38,6 +38,7 @@
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/strings/split.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/tablet/tablet.pb.h"
#include "kudu/tools/color.h"
@@ -53,10 +54,19 @@
} \
} while (0)
+#define STR_FLAGS_CATEGORY_TIME_SOURCE "time_source"
+#define STR_FLAGS_CATEGORY_UNUSUAL "unusual"
+
DEFINE_bool(checksum_scan, false,
"Perform a checksum scan on data in the cluster.");
DEFINE_int32(fetch_info_concurrency, 20,
"Number of threads to fetch info concurrently.");
+DEFINE_string(flags_categories_to_check, STR_FLAGS_CATEGORY_TIME_SOURCE,
+ "Comma-separated list of flag categories to check for divergence "
+ "across the cluster; default is "
+ STR_FLAGS_CATEGORY_TIME_SOURCE "; available categories are "
+ STR_FLAGS_CATEGORY_TIME_SOURCE ","
+ STR_FLAGS_CATEGORY_UNUSUAL ".");
DEFINE_string(ksck_format, "plain_concise",
"Output format for ksck. Available options are 'plain_concise', "
@@ -79,11 +89,14 @@ using kudu::cluster_summary::ServerHealth;
using kudu::cluster_summary::ServerHealthSummary;
using kudu::cluster_summary::TableSummary;
using kudu::cluster_summary::TabletSummary;
+using kudu::server::GetFlagsResponsePB;
using std::atomic;
using std::cout;
using std::ostream;
using std::ostringstream;
+using std::pair;
+using std::set;
using std::shared_ptr;
using std::string;
using std::vector;
@@ -92,6 +105,23 @@ using strings::Substitute;
namespace kudu {
namespace tools {
+DEFINE_validator(flags_categories_to_check,
+ [](const char* /* flag_name */, const string& value) {
+ vector<string> categories;
+ SplitStringUsing(value, ",", &categories);
+ for (const auto& cat : categories) {
+ if (cat.empty() || StringToFlagsCategory(cat, nullptr).ok()) {
+ continue;
+ }
+ LOG(ERROR) << Substitute("unknown flag category: '$0' "
+ "(expecting comma-separated list built out of "
+ STR_FLAGS_CATEGORY_TIME_SOURCE ", "
+ STR_FLAGS_CATEGORY_UNUSUAL ")", cat);
+ return false;
+ }
+ return true;
+});
+
namespace {
void BuildConsensusStateForConfigMember(const consensus::ConsensusStatePB& cstate,
ConsensusState* ksck_cstate) {
@@ -159,6 +189,70 @@ std::ostream& operator<<(std::ostream& lhs, KsckFetchState state) {
return lhs;
}
+const FlagsFetchFilter& GetFlagsCategoryFilter(FlagsCategory category) {
+ // NOTE: using double braces for std::array aggregate initialization.
+ static const std::array<FlagsFetchFilter, FlagsCategory::ARRAY_SIZE> kFilters { {
+ {
+ // FlagsCategory::TIME_SOURCE
+ { "time_source", "builtin_ntp_servers", },
+ {}
+ },
+ {
+ // FlagsCategory::UNUSUAL
+ {},
+ { "experimental", "hidden", "unsafe" }
+ },
+ } };
+ DCHECK_GE(category, FlagsCategory::MIN);
+ DCHECK_LE(category, FlagsCategory::MAX);
+ return kFilters[category];
+}
+
+const char* FlagsCategoryToString(FlagsCategory category) {
+ static constexpr const char* const kCategoryTimeSource =
+ STR_FLAGS_CATEGORY_TIME_SOURCE;
+ static constexpr const char* const kCategoryUnusual =
+ STR_FLAGS_CATEGORY_UNUSUAL;
+ switch (category) {
+ case FlagsCategory::TIME_SOURCE:
+ return kCategoryTimeSource;
+ case FlagsCategory::UNUSUAL:
+ return kCategoryUnusual;
+ }
+ return "unknown";
+}
+
+Status StringToFlagsCategory(const string& str, FlagsCategory* category) {
+ if (boost::iequals(str, STR_FLAGS_CATEGORY_TIME_SOURCE)) {
+ if (category) {
+ *category = FlagsCategory::TIME_SOURCE;
+ }
+ return Status::OK();
+ }
+ if (boost::iequals(str, STR_FLAGS_CATEGORY_UNUSUAL)) {
+ if (category) {
+ *category = FlagsCategory::UNUSUAL;
+ }
+ return Status::OK();
+ }
+ return Status::InvalidArgument(Substitute("$0: unknown flags category", str));
+}
+
+Status StringToFlagsCategories(const std::string& str,
+ vector<FlagsCategory>* categories) {
+ DCHECK(categories);
+ vector<string> categories_str(strings::Split(str, ",", strings::SkipEmpty()));
+ for (const auto& str : categories_str) {
+ FlagsCategory cat;
+ RETURN_NOT_OK(StringToFlagsCategory(str, &cat));
+ categories->push_back(cat);
+ }
+ std::sort(categories->begin(), categories->end());
+ categories->erase(std::unique(categories->begin(), categories->end()),
+ categories->end());
+ return Status::OK();
+}
+
Ksck::Ksck(shared_ptr<KsckCluster> cluster, ostream* out)
: cluster_(std::move(cluster)),
out_(out == nullptr ? &std::cout : out) {
@@ -180,35 +274,38 @@ Status Ksck::CheckMasterHealth() {
vector<ServerHealthSummary> master_summaries;
simple_spinlock master_summaries_lock;
+ vector<FlagsCategory> flags_categories_to_fetch = { FlagsCategory::UNUSUAL };
+ RETURN_NOT_OK(StringToFlagsCategories(FLAGS_flags_categories_to_check,
+ &flags_categories_to_fetch));
for (const auto& master : cluster_->masters()) {
RETURN_NOT_OK(pool_->SubmitFunc([&]() {
- ServerHealthSummary sh;
- Status s = master->FetchInfo().AndThen([&]() {
- return master->FetchConsensusState();
- });
- sh.uuid = master->uuid();
- sh.address = master->address();
- sh.version = master->version();
- sh.status = s;
- if (!s.ok()) {
- if (IsNotAuthorizedMethodAccess(s)) {
- sh.health = ServerHealth::UNAUTHORIZED;
- ++unauthorized_masters;
- } else {
- sh.health = ServerHealth::UNAVAILABLE;
- }
- ++bad_masters;
+ ServerHealthSummary sh;
+ Status s = master->FetchInfo().AndThen([&]() {
+ return master->FetchConsensusState();
+ });
+ sh.uuid = master->uuid();
+ sh.address = master->address();
+ sh.version = master->version();
+ sh.status = s;
+ if (!s.ok()) {
+ if (IsNotAuthorizedMethodAccess(s)) {
+ sh.health = ServerHealth::UNAUTHORIZED;
+ ++unauthorized_masters;
+ } else {
+ sh.health = ServerHealth::UNAVAILABLE;
}
+ ++bad_masters;
+ }
- {
- std::lock_guard<simple_spinlock> lock(master_summaries_lock);
- master_summaries.emplace_back(std::move(sh));
- }
+ {
+ std::lock_guard<simple_spinlock> lock(master_summaries_lock);
+ master_summaries.emplace_back(std::move(sh));
+ }
- // Fetch the flags information.
- // Flag retrieval is not supported by older versions; failure is tracked in
- // CheckMasterUnusualFlags().
- ignore_result(master->FetchUnusualFlags());
+ // Fetch the flags information in every requested category.
+ // Flag retrieval is not supported by older versions; failure is tracked
+ // in CheckTabletServer{Unusual,Diverged}Flags().
+ ignore_result(master->FetchFlags(flags_categories_to_fetch));
}));
}
pool_->Wait();
@@ -273,44 +370,83 @@ Status Ksck::CheckMasterConsensus() {
return Status::OK();
}
-void Ksck::AddFlagsToFlagMaps(const server::GetFlagsResponsePB& flags,
+void Ksck::AddFlagsToFlagMaps(const GetFlagsResponsePB& flags,
const string& server_address,
KsckFlagToServersMap* flags_to_servers_map,
KsckFlagTagsMap* flag_tags_map) {
- CHECK(flags_to_servers_map);
- CHECK(flag_tags_map);
+ DCHECK(flags_to_servers_map);
for (const auto& f : flags.flags()) {
- const std::pair<string, string> key(f.name(), f.value());
+ const pair<string, string> key(f.name(), f.value());
if (!InsertIfNotPresent(flags_to_servers_map, key, { server_address })) {
FindOrDieNoPrint(*flags_to_servers_map, key).push_back(server_address);
}
- InsertIfNotPresent(flag_tags_map, f.name(), JoinStrings(f.tags(), ","));
+ if (flag_tags_map != nullptr) {
+ InsertIfNotPresent(flag_tags_map, f.name(), JoinStrings(f.tags(), ","));
+ }
}
}
Status Ksck::CheckMasterUnusualFlags() {
- int bad_masters = 0;
- Status last_error = Status::OK();
+ size_t bad_servers = 0;
for (const auto& master : cluster_->masters()) {
- if (!master->unusual_flags()) {
- bad_masters++;
+ const auto& unusual_flags = master->flags(FlagsCategory::UNUSUAL);
+ if (!unusual_flags) {
+ ++bad_servers;
continue;
}
- AddFlagsToFlagMaps(*master->unusual_flags(),
+ AddFlagsToFlagMaps(*unusual_flags,
master->address(),
- &results_.master_flag_to_servers_map,
- &results_.master_flag_tags_map);
+ &results_.master_unusual_flag_to_servers_map,
+ &results_.master_unusual_flag_tags_map);
}
- if (!results_.master_flag_to_servers_map.empty()) {
+ if (!results_.master_unusual_flag_to_servers_map.empty()) {
results_.warning_messages.push_back(Status::ConfigurationError(
"Some masters have unsafe, experimental, or hidden flags set"));
}
- if (bad_masters > 0) {
- return Status::Incomplete(
- Substitute("$0 of $1 masters' flags were not available",
- bad_masters, cluster_->masters().size()));
+ if (bad_servers > 0) {
+ return Status::Incomplete(Substitute(
+ "$0 of $1 masters were not available to retrieve unusual flags",
+ bad_servers, cluster_->masters().size()));
+ }
+ return Status::OK();
+}
+
+Status Ksck::CheckMasterDivergedFlags() {
+ vector<FlagsCategory> flags_categories;
+ RETURN_NOT_OK(StringToFlagsCategories(FLAGS_flags_categories_to_check,
+ &flags_categories));
+ for (const auto cat : flags_categories) {
+ KsckFlagToServersMap servers_by_flag;
+ size_t bad_servers = 0;
+ for (const auto& master : cluster_->masters()) {
+ const auto& flags = master->flags(cat);
+ if (!flags) {
+ ++bad_servers;
+ continue;
+ }
+ AddFlagsToFlagMaps(*flags, master->address(), &servers_by_flag);
+ AddFlagsToFlagMaps(*flags,
+ master->address(),
+ &results_.master_checked_flag_to_servers_map);
+ }
+
+ for (const auto& e : servers_by_flag) {
+ if (e.second.size() + bad_servers == cluster_->masters().size()) {
+ continue;
+ }
+ results_.warning_messages.push_back(Status::ConfigurationError(
+ Substitute("Different masters have different settings for same "
+ "flags of checked category '$0'",
+ FlagsCategoryToString(cat))));
+ break;
+ }
+ if (bad_servers > 0) {
+ return Status::Incomplete(Substitute(
+ "$0 of $1 masters were not available to retrieve $2 category flags",
+ bad_servers, cluster_->masters().size(), FlagsCategoryToString(cat)));
+ }
}
return Status::OK();
}
@@ -339,6 +475,9 @@ Status Ksck::FetchInfoFromTabletServers() {
vector<ServerHealthSummary> tablet_server_summaries;
simple_spinlock tablet_server_summaries_lock;
+ vector<FlagsCategory> flags_categories_to_fetch = { FlagsCategory::UNUSUAL };
+ RETURN_NOT_OK(StringToFlagsCategories(FLAGS_flags_categories_to_check,
+ &flags_categories_to_fetch));
for (const auto& entry : cluster_->tablet_servers()) {
const auto& ts = entry.second;
RETURN_NOT_OK(pool_->SubmitFunc([&]() {
@@ -371,10 +510,10 @@ Status Ksck::FetchInfoFromTabletServers() {
tablet_server_summaries.push_back(std::move(summary));
}
- // Fetch the flags information.
- // Flag retrieval is not supported by older versions; failure is tracked in
- // CheckTabletServerUnusualFlags().
- ignore_result(ts->FetchUnusualFlags());
+ // Fetch the flags information in every requested category.
+ // Flag retrieval is not supported by older versions; failure is tracked
+ // in CheckTabletServer{Unusual,Diverged}Flags().
+ ignore_result(ts->FetchFlags(flags_categories_to_fetch));
}));
}
pool_->Wait();
@@ -439,7 +578,9 @@ Status Ksck::Run() {
PUSH_PREPEND_NOT_OK(CheckMasterConsensus(), results_.error_messages,
"master consensus error");
PUSH_PREPEND_NOT_OK(CheckMasterUnusualFlags(), results_.warning_messages,
- "master flag check error");
+ "master unusual flags check error");
+ PUSH_PREPEND_NOT_OK(CheckMasterDivergedFlags(), results_.warning_messages,
+ "master diverged flags check error");
// CheckClusterRunning and FetchTableAndTabletInfo must succeed for
// subsequent checks to be runnable.
@@ -460,10 +601,15 @@ Status Ksck::Run() {
PUSH_PREPEND_NOT_OK(FetchInfoFromTabletServers(), results_.error_messages,
"error fetching info from tablet servers");
PUSH_PREPEND_NOT_OK(CheckTabletServerUnusualFlags(), results_.warning_messages,
- "tserver flag check error");
+ "tserver unusual flags check error");
+ PUSH_PREPEND_NOT_OK(CheckTabletServerDivergedFlags(), results_.warning_messages,
+ "tserver diverged flags check error");
PUSH_PREPEND_NOT_OK(CheckServerVersions(), results_.warning_messages,
"version check error");
+ PUSH_PREPEND_NOT_OK(CheckDivergedFlags(), results_.warning_messages,
+ "diverged flags (both masters and tservers) check error");
+
PUSH_PREPEND_NOT_OK(CheckTablesConsistency(), results_.error_messages,
"table consistency check error");
@@ -492,28 +638,112 @@ Status Ksck::Run() {
}
Status Ksck::CheckTabletServerUnusualFlags() {
- int bad_tservers = 0;
+ int bad_servers = 0;
for (const auto& uuid_and_ts : cluster_->tablet_servers()) {
const auto& tserver = uuid_and_ts.second;
- if (!tserver->unusual_flags()) {
- bad_tservers++;
+ const auto& unusual_flags = tserver->flags(FlagsCategory::UNUSUAL);
+ if (!unusual_flags) {
+ ++bad_servers;
continue;
}
- AddFlagsToFlagMaps(*tserver->unusual_flags(),
+ AddFlagsToFlagMaps(*unusual_flags,
tserver->address(),
- &results_.tserver_flag_to_servers_map,
- &results_.tserver_flag_tags_map);
+ &results_.tserver_unusual_flag_to_servers_map,
+ &results_.tserver_unusual_flag_tags_map);
}
- if (!results_.tserver_flag_to_servers_map.empty()) {
+ if (!results_.tserver_unusual_flag_to_servers_map.empty()) {
results_.warning_messages.push_back(Status::ConfigurationError(
"Some tablet servers have unsafe, experimental, or hidden flags set"));
}
- if (bad_tservers > 0) {
- return Status::Incomplete(
- Substitute("$0 of $1 tservers' flags were not available",
- bad_tservers, cluster_->tablet_servers().size()));
+ if (bad_servers > 0) {
+ return Status::Incomplete(Substitute(
+ "$0 of $1 tservers were not available to retrieve unusual flags",
+ bad_servers, cluster_->tablet_servers().size()));
+ }
+ return Status::OK();
+}
+
+Status Ksck::CheckTabletServerDivergedFlags() {
+ vector<FlagsCategory> flags_categories;
+ RETURN_NOT_OK(StringToFlagsCategories(FLAGS_flags_categories_to_check,
+ &flags_categories));
+ for (const auto cat : flags_categories) {
+ KsckFlagToServersMap servers_by_flag;
+ size_t bad_servers = 0;
+ for (const auto& uuid_and_ts : cluster_->tablet_servers()) {
+ const auto& tserver = uuid_and_ts.second;
+ const auto& flags = tserver->flags(cat);
+ if (!flags) {
+ ++bad_servers;
+ continue;
+ }
+ AddFlagsToFlagMaps(*flags, tserver->address(), &servers_by_flag);
+ AddFlagsToFlagMaps(*flags,
+ tserver->address(),
+ &results_.tserver_checked_flag_to_servers_map);
+ }
+ for (const auto& e : servers_by_flag) {
+ if (e.second.size() + bad_servers == cluster_->tablet_servers().size()) {
+ continue;
+ }
+ results_.warning_messages.push_back(Status::ConfigurationError(
+ Substitute("Different tservers have different settings for same "
+ "flags of checked category '$0'",
+ FlagsCategoryToString(cat))));
+ break;
+ }
+ if (bad_servers > 0) {
+ return Status::Incomplete(Substitute(
+ "$0 of $1 tservers were not available to retrieve $2 category flags",
+ bad_servers,
+ cluster_->tablet_servers().size(),
+ FlagsCategoryToString(cat)));
+ }
+ }
+ return Status::OK();
+}
+
+Status Ksck::CheckDivergedFlags() {
+ set<KsckFlag> masters_flags;
+ for (const auto& elem : results_.master_checked_flag_to_servers_map) {
+ InsertOrDieNoPrint(&masters_flags, elem.first);
+ }
+ set<KsckFlag> tservers_flags;
+ for (const auto& elem : results_.tserver_checked_flag_to_servers_map) {
+ InsertOrDieNoPrint(&tservers_flags, elem.first);
+ }
+
+ vector<KsckFlag> symm_diff;
+ std::set_symmetric_difference(masters_flags.begin(),
+ masters_flags.end(),
+ tservers_flags.begin(),
+ tservers_flags.end(),
+ back_inserter(symm_diff));
+ if (!symm_diff.empty()) {
+ for (const auto& f : symm_diff) {
+ {
+ const auto* e = FindOrNull(results_.master_checked_flag_to_servers_map, f);
+ if (e) {
+ InsertOrDieNoPrint(&results_.master_diverged_flag_to_servers_map, f, *e);
+ continue;
+ }
+ }
+ {
+ const auto* e = FindOrNull(results_.tserver_checked_flag_to_servers_map, f);
+ if (e) {
+ InsertOrDieNoPrint(&results_.tserver_diverged_flag_to_servers_map, f, *e);
+ continue;
+ }
+ }
+ // The flag/value pair must be either of masters' or tservers'.
+ LOG(DFATAL) << "found neither masters' or tservers' flag: " << f.first;
+ }
+
+ results_.warning_messages.push_back(Status::ConfigurationError(
+ "Same flags have different values between masters and tservers "
+ "for at least one checked flag category"));
}
return Status::OK();
}
@@ -686,7 +916,7 @@ HealthCheckResult Ksck::VerifyTablet(const shared_ptr<KsckTablet>& tablet,
}
// Organize consensus info for each replica.
- std::pair<string, string> tablet_key = std::make_pair(ts->uuid(), tablet->id());
+ pair<string, string> tablet_key = std::make_pair(ts->uuid(), tablet->id());
if (ContainsKey(ts->tablet_consensus_state_map(), tablet_key)) {
const auto& cstate = FindOrDieNoPrint(ts->tablet_consensus_state_map(), tablet_key);
ConsensusState ksck_cstate;
diff --git a/src/kudu/tools/ksck.h b/src/kudu/tools/ksck.h
index 2c3b63e..5dee873 100644
--- a/src/kudu/tools/ksck.h
+++ b/src/kudu/tools/ksck.h
@@ -18,6 +18,7 @@
// Ksck, a tool to run a Kudu System Check.
#pragma once
+#include <array>
#include <atomic>
#include <cstdint>
#include <iosfwd>
@@ -174,6 +175,57 @@ enum class KsckFetchState {
// Required for logging in case of CHECK failures.
std::ostream& operator<<(std::ostream& lhs, KsckFetchState state);
+// Enum representing logical categories of flags used in a Kudu cluster.
+enum FlagsCategory {
+ // Flags specific to the time source, clock, and alike.
+ TIME_SOURCE = 0,
+
+ // Flags tagged hidden, experimental, or unsafe.
+ UNUSUAL = 1,
+
+ // Utility members used for range-related expressions.
+ // NOTE: update the MAX member upon updates
+ MIN = 0,
+ MAX = UNUSUAL,
+ ARRAY_SIZE = MAX + 1,
+};
+
+// Information on flags fetched using GetFlags() RPC. Consists of the status of
+// the GetFlags() operation and the result flags wrapped into boost::optional,
+// where the latter is boost::none unless the RPC was successful.
+struct FetchedFlags {
+ KsckFetchState state = KsckFetchState::UNINITIALIZED;
+ boost::optional<server::GetFlagsResponsePB> flags;
+};
+
+// Flags retrieved using GetFlags(), indexed by FlagsCategory.
+typedef std::array<FetchedFlags, FlagsCategory::ARRAY_SIZE> FetchedFlagsByCategory;
+
+// Structure to represent a filter to build appropriate request for GetFlags()
+// RPC.
+struct FlagsFetchFilter {
+ const std::vector<std::string> flags;
+ const std::vector<std::string> tags;
+};
+
+// Get filter to build GetFlags() RPC for the specified flags category.
+const FlagsFetchFilter& GetFlagsCategoryFilter(FlagsCategory category);
+
+// Get string representation for the specified flag category.
+const char* FlagsCategoryToString(FlagsCategory category);
+
+// Convert string representation of flag category into corresponding enum field.
+// On success, returns Status::OK() and outputs the result category into the
+// 'category' output parameter. On failure returns Status::InvalidParameter().
+Status StringToFlagsCategory(const std::string& str,
+ FlagsCategory* category);
+
+// Convert comma-separated string into vector of flag categories. The result
+// is appended to the 'categories' output parameter. In addition, this function
+// removes duplicates in resulting 'categories'.
+Status StringToFlagsCategories(const std::string& str,
+ std::vector<FlagsCategory>* categories);
+
// The following three classes must be extended in order to communicate with their respective
// components. The two main use cases envisioned for this are:
// - To be able to mock a cluster to more easily test the ksck checks.
@@ -183,8 +235,8 @@ std::ostream& operator<<(std::ostream& lhs, KsckFetchState state);
class KsckMaster {
public:
explicit KsckMaster(std::string address) :
- address_(std::move(address)),
- uuid_(strings::Substitute("$0 ($1)", kDummyUuid, address_)) {}
+ address_(std::move(address)),
+ uuid_(strings::Substitute("$0 ($1)", kDummyUuid, address_)) {}
virtual ~KsckMaster() = default;
@@ -196,9 +248,8 @@ class KsckMaster {
// Connects to the master and populates the consensus map.
virtual Status FetchConsensusState() = 0;
- // Retrieves "unusual" flags from the KsckMaster.
- // "Unusual" flags ares ones tagged hidden, experimental, or unsafe.
- virtual Status FetchUnusualFlags() = 0;
+ // Fetch flags for the requested categories.
+ virtual Status FetchFlags(const std::vector<FlagsCategory>& categories) = 0;
// Since masters are provided by address, FetchInfo() must be called before
// calling this method.
@@ -221,9 +272,12 @@ class KsckMaster {
return cstate_;
}
- virtual const boost::optional<server::GetFlagsResponsePB>& unusual_flags() const {
- CHECK_NE(KsckFetchState::UNINITIALIZED, unusual_flags_state_);
- return unusual_flags_;
+ virtual const boost::optional<server::GetFlagsResponsePB>& flags(
+ FlagsCategory category) const {
+ CHECK_GE(category, FlagsCategory::MIN);
+ CHECK_LE(category, FlagsCategory::MAX);
+ CHECK_NE(KsckFetchState::UNINITIALIZED, flags_by_category_[category].state);
+ return flags_by_category_[category].flags;
}
std::string ToString() const {
@@ -252,15 +306,12 @@ class KsckMaster {
// May be none if fetching info from the master fails.
boost::optional<std::string> version_;
+ // Fetched flags indexed by category.
+ FetchedFlagsByCategory flags_by_category_;
+
// May be none if consensus state fetch fails.
boost::optional<consensus::ConsensusStatePB> cstate_;
- // unusual_flags_state_ reflects whether the fetch of the non-critical flags
- // info has been done, and if it succeeded or failed.
- KsckFetchState unusual_flags_state_ = KsckFetchState::UNINITIALIZED;
- // May be none if flag fetch fails.
- boost::optional<server::GetFlagsResponsePB> unusual_flags_;
-
private:
DISALLOW_COPY_AND_ASSIGN(KsckMaster);
};
@@ -272,14 +323,15 @@ class KsckTabletServer {
typedef std::unordered_map<std::string, tablet::TabletStatusPB> TabletStatusMap;
// Map from (tserver id, tablet id) to tablet consensus information.
- typedef std::map
- <std::pair<std::string, std::string>, consensus::ConsensusStatePB> TabletConsensusStateMap;
+ typedef std::map<std::pair<std::string, std::string>,
+ consensus::ConsensusStatePB>
+ TabletConsensusStateMap;
explicit KsckTabletServer(std::string uuid, std::string location = "")
: uuid_(std::move(uuid)),
location_(std::move(location)) {}
- virtual ~KsckTabletServer() { }
+ virtual ~KsckTabletServer() {}
// Connects to the configured tablet server and populates the fields of this class. 'health' must
// not be nullptr.
@@ -296,9 +348,8 @@ class KsckTabletServer {
// Otherwise 'health' will be UNAVAILABLE
virtual Status FetchConsensusState(cluster_summary::ServerHealth* health) = 0;
- // Retrieves "unusual" flags from the KsckTabletServer.
- // "Unusual" flags ares ones tagged hidden, experimental, or unsafe.
- virtual Status FetchUnusualFlags() = 0;
+ // Fetches flags for the requested categories.
+ virtual Status FetchFlags(const std::vector<FlagsCategory>& categories) = 0;
// Fetches and updates the current timestamp from the tablet server.
virtual void FetchCurrentTimestampAsync() = 0;
@@ -354,9 +405,12 @@ class KsckTabletServer {
return version_;
}
- virtual const boost::optional<server::GetFlagsResponsePB>& unusual_flags() const {
- CHECK_NE(KsckFetchState::UNINITIALIZED, unusual_flags_state_);
- return unusual_flags_;
+ virtual const boost::optional<server::GetFlagsResponsePB>& flags(
+ FlagsCategory category) const {
+ DCHECK_GE(category, FlagsCategory::MIN);
+ DCHECK_LE(category, FlagsCategory::MAX);
+ CHECK_NE(KsckFetchState::UNINITIALIZED, flags_by_category_[category].state);
+ return flags_by_category_[category].flags;
}
virtual const boost::optional<cluster_summary::QuiescingInfo>& quiescing_info() const {
@@ -382,25 +436,21 @@ class KsckTabletServer {
// it succeeded or failed.
KsckFetchState state_ = KsckFetchState::UNINITIALIZED;
- TabletStatusMap tablet_status_map_;
- TabletConsensusStateMap tablet_consensus_state_map_;
-
// May be none if fetching info from the tablet server fails.
boost::optional<std::string> version_;
- // unusual_flags_state_ reflects whether the fetch of the non-critical flags
- // info has been done, and if it succeeded or failed.
- KsckFetchState unusual_flags_state_ = KsckFetchState::UNINITIALIZED;
+ // Fetched flags indexed by category.
+ FetchedFlagsByCategory flags_by_category_;
- // May be none if flag fetch fails.
- boost::optional<server::GetFlagsResponsePB> unusual_flags_;
+ TabletStatusMap tablet_status_map_;
+ TabletConsensusStateMap tablet_consensus_state_map_;
// May be none if the quiescing request fails.
boost::optional<cluster_summary::QuiescingInfo> quiescing_info_;
- std::atomic<uint64_t> timestamp_;
const std::string uuid_;
std::string location_;
+ std::atomic<uint64_t> timestamp_;
private:
DISALLOW_COPY_AND_ASSIGN(KsckTabletServer);
@@ -562,6 +612,10 @@ class Ksck {
// Must first call CheckMasterHealth().
Status CheckMasterUnusualFlags();
+ // Check for the consistency of flag values across all masters in the cluster.
+ // Must first call CheckMasterHealth().
+ Status CheckMasterDivergedFlags();
+
// Verifies that it can connect to the cluster, i.e. that it can contact a
// leader master.
Status CheckClusterRunning();
@@ -581,6 +635,17 @@ class Ksck {
// Must first call FetchInfoFromTabletServers().
Status CheckTabletServerUnusualFlags();
+ // Check for the consistency of flag values across all tablet servers
+ // in the cluster.
+ // Must first call FetchInfoFromTabletServers().
+ Status CheckTabletServerDivergedFlags();
+
+ // Check for the consistency of flag values across all tablet servers and
+ // masters in the cluster.
+ // Must first call CheckMasterDivergedFlags() and
+ // CheckTabletServerDivergedFlags().
+ Status CheckDivergedFlags();
+
// Check for version inconsistencies among all servers.
Status CheckServerVersions();
@@ -610,13 +675,14 @@ class Ksck {
private:
friend class KsckTest;
- // Accumulate information about flags from a server into a FlagToServersMap and
- // a FlagTagsMap.
- // 'flags_to_server_map' and 'flag_tags_map' must not be null.
- void AddFlagsToFlagMaps(const server::GetFlagsResponsePB& flags,
- const std::string& server_address,
- KsckFlagToServersMap* flags_to_servers_map,
- KsckFlagTagsMap* flag_tags_map);
+ // Accumulate information about flags from a server into a FlagToServersMap
+ // and, optionally, flag's tags into a FlagTagsMap. 'flags_to_server_map' must
+ // not be null; 'flag_tags_map' may be null: in such case no information on
+ // flag's tags is accumulated.
+ static void AddFlagsToFlagMaps(const server::GetFlagsResponsePB& flags,
+ const std::string& server_address,
+ KsckFlagToServersMap* flags_to_servers_map,
+ KsckFlagTagsMap* flag_tags_map = nullptr);
bool VerifyTable(const std::shared_ptr<KsckTable>& table);
diff --git a/src/kudu/tools/ksck_remote.cc b/src/kudu/tools/ksck_remote.cc
index 1afd91a..2804eca 100644
--- a/src/kudu/tools/ksck_remote.cc
+++ b/src/kudu/tools/ksck_remote.cc
@@ -20,7 +20,6 @@
#include <atomic>
#include <cstdint>
#include <functional>
-#include <initializer_list>
#include <map>
#include <mutex>
#include <ostream>
@@ -95,6 +94,9 @@ using kudu::master::TServerStatePB;
using kudu::rpc::Messenger;
using kudu::rpc::MessengerBuilder;
using kudu::rpc::RpcController;
+using kudu::server::GenericServiceProxy;
+using kudu::server::GetFlagsRequestPB;
+using kudu::server::GetFlagsResponsePB;
using std::shared_ptr;
using std::string;
using std::unique_ptr;
@@ -111,17 +113,24 @@ MonoDelta GetDefaultTimeout() {
return MonoDelta::FromMilliseconds(FLAGS_timeout_ms);
}
-// Common flag-fetching routine for masters and tablet servers.
-Status FetchUnusualFlagsCommon(const shared_ptr<server::GenericServiceProxy>& proxy,
- server::GetFlagsResponsePB* resp) {
- server::GetFlagsRequestPB req;
- RpcController rpc;
- rpc.set_timeout(GetDefaultTimeout());
- for (const string& tag : { "experimental", "hidden", "unsafe" }) {
+// Common flag-fetching routine. Fetches flags for the specified category,
+// given service proxy object.
+Status FetchCategoryFlags(FlagsCategory category,
+ const shared_ptr<GenericServiceProxy>& proxy,
+ GetFlagsResponsePB* resp) {
+ const auto& filter = GetFlagsCategoryFilter(category);
+ GetFlagsRequestPB req;
+ for (const auto& flag : filter.flags) {
+ req.add_flags(flag);
+ }
+ for (const auto& tag : filter.tags) {
req.add_tags(tag);
}
- return proxy->GetFlags(req, resp, &rpc);
+ RpcController ctl;
+ ctl.set_timeout(GetDefaultTimeout());
+ return proxy->GetFlags(req, resp, &ctl);
}
+
} // anonymous namespace
Status RemoteKsckMaster::Init() {
@@ -174,16 +183,20 @@ Status RemoteKsckMaster::FetchConsensusState() {
return Status::OK();
}
-Status RemoteKsckMaster::FetchUnusualFlags() {
- server::GetFlagsResponsePB resp;
- Status s = FetchUnusualFlagsCommon(generic_proxy_, &resp);
- if (!s.ok()) {
- unusual_flags_state_ = KsckFetchState::FETCH_FAILED;
- } else {
- unusual_flags_state_ = KsckFetchState::FETCHED;
- unusual_flags_ = std::move(resp);
+Status RemoteKsckMaster::FetchFlags(const vector<FlagsCategory>& categories) {
+ Status result;
+ for (auto cat : categories) {
+ GetFlagsResponsePB resp;
+ const auto s = FetchCategoryFlags(cat, generic_proxy_, &resp);
+ if (!s.ok()) {
+ flags_by_category_[cat].state = KsckFetchState::FETCH_FAILED;
+ result = result.ok() ? s : result.CloneAndAppend(s.message());
+ } else {
+ flags_by_category_[cat].state = KsckFetchState::FETCHED;
+ flags_by_category_[cat].flags = std::move(resp);
+ }
}
- return s;
+ return result;
}
Status RemoteKsckTabletServer::Init() {
@@ -323,16 +336,20 @@ Status RemoteKsckTabletServer::FetchConsensusState(ServerHealth* health) {
return Status::OK();
}
-Status RemoteKsckTabletServer::FetchUnusualFlags() {
- server::GetFlagsResponsePB resp;
- Status s = FetchUnusualFlagsCommon(generic_proxy_, &resp);
- if (!s.ok()) {
- unusual_flags_state_ = KsckFetchState::FETCH_FAILED;
- } else {
- unusual_flags_state_ = KsckFetchState::FETCHED;
- unusual_flags_ = std::move(resp);
+Status RemoteKsckTabletServer::FetchFlags(const vector<FlagsCategory>& categories) {
+ Status result;
+ for (auto cat : categories) {
+ GetFlagsResponsePB resp;
+ const auto s = FetchCategoryFlags(cat, generic_proxy_, &resp);
+ if (!s.ok()) {
+ flags_by_category_[cat].state = KsckFetchState::FETCH_FAILED;
+ result = result.ok() ? s : result.CloneAndAppend(s.message());
+ } else {
+ flags_by_category_[cat].state = KsckFetchState::FETCHED;
+ flags_by_category_[cat].flags = std::move(resp);
+ }
}
- return s;
+ return result;
}
class ChecksumStepper;
diff --git a/src/kudu/tools/ksck_remote.h b/src/kudu/tools/ksck_remote.h
index c1dc4ab..b82e6f4 100644
--- a/src/kudu/tools/ksck_remote.h
+++ b/src/kudu/tools/ksck_remote.h
@@ -82,7 +82,7 @@ class RemoteKsckMaster : public KsckMaster {
// Gathers consensus state for the master tablet.
Status FetchConsensusState() override;
- Status FetchUnusualFlags() override;
+ Status FetchFlags(const std::vector<FlagsCategory>& categories) override;
private:
std::shared_ptr<rpc::Messenger> messenger_;
@@ -111,7 +111,7 @@ class RemoteKsckTabletServer : public KsckTabletServer,
Status FetchConsensusState(cluster_summary::ServerHealth* health) override;
- Status FetchUnusualFlags() override;
+ Status FetchFlags(const std::vector<FlagsCategory>& categories) override;
void FetchCurrentTimestampAsync() override;
Status FetchCurrentTimestamp() override;
diff --git a/src/kudu/tools/ksck_results.cc b/src/kudu/tools/ksck_results.cc
index 16a9506..eebd5e7 100644
--- a/src/kudu/tools/ksck_results.cc
+++ b/src/kudu/tools/ksck_results.cc
@@ -172,6 +172,12 @@ string ServerCsv(int server_count, const vector<string>& servers) {
} // anonymous namespace
Status KsckResults::PrintTo(PrintMode mode, int sections, ostream& out) {
+ static const char* const kMsgCheckedFlags =
+ "Flags of checked categories for $0:";
+ static const char* const kMsgUnusualFlags = "Unusual flags for $0:";
+ static const char* const kMsgDivergedFlags =
+ "Flags of checked categories for $0 diverging from $1 flags:";
+
if (mode == PrintMode::JSON_PRETTY || mode == PrintMode::JSON_COMPACT) {
return PrintJsonTo(mode, sections, out);
}
@@ -192,12 +198,35 @@ Status KsckResults::PrintTo(PrintMode mode, int sections, ostream& out) {
}
out << endl;
- RETURN_NOT_OK(PrintFlagTable(ServerType::MASTER,
- cluster_status.master_summaries.size(),
- master_flag_to_servers_map,
- master_flag_tags_map,
- out));
- if (!master_flag_to_servers_map.empty()) {
+ if (!master_unusual_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgUnusualFlags,
+ ServerTypeToString(ServerType::MASTER)) << endl;
+ RETURN_NOT_OK(PrintTaggedFlagTable(ServerType::MASTER,
+ cluster_status.master_summaries.size(),
+ master_unusual_flag_to_servers_map,
+ master_unusual_flag_tags_map,
+ out));
+ out << endl;
+ }
+
+ if (!master_checked_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgCheckedFlags,
+ ServerTypeToString(ServerType::MASTER)) << endl;
+ RETURN_NOT_OK(PrintFlagTable(ServerType::MASTER,
+ cluster_status.master_summaries.size(),
+ master_checked_flag_to_servers_map,
+ out));
+ out << endl;
+ }
+
+ if (!master_diverged_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgDivergedFlags,
+ ServerTypeToString(ServerType::MASTER),
+ ServerTypeToString(ServerType::TABLET_SERVER)) << endl;
+ RETURN_NOT_OK(PrintFlagTable(ServerType::MASTER,
+ cluster_status.master_summaries.size(),
+ master_diverged_flag_to_servers_map,
+ out));
out << endl;
}
}
@@ -221,12 +250,35 @@ Status KsckResults::PrintTo(PrintMode mode, int sections, ostream& out) {
out << endl;
}
- RETURN_NOT_OK(PrintFlagTable(ServerType::TABLET_SERVER,
- cluster_status.tserver_summaries.size(),
- tserver_flag_to_servers_map,
- tserver_flag_tags_map,
- out));
- if (!tserver_flag_to_servers_map.empty()) {
+ if (!tserver_unusual_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgUnusualFlags,
+ ServerTypeToString(ServerType::TABLET_SERVER)) << endl;
+ RETURN_NOT_OK(PrintTaggedFlagTable(ServerType::TABLET_SERVER,
+ cluster_status.tserver_summaries.size(),
+ tserver_unusual_flag_to_servers_map,
+ tserver_unusual_flag_tags_map,
+ out));
+ out << endl;
+ }
+
+ if (!tserver_checked_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgCheckedFlags,
+ ServerTypeToString(ServerType::TABLET_SERVER)) << endl;
+ RETURN_NOT_OK(PrintFlagTable(ServerType::TABLET_SERVER,
+ cluster_status.tserver_summaries.size(),
+ tserver_checked_flag_to_servers_map,
+ out));
+ out << endl;
+ }
+
+ if (!tserver_diverged_flag_to_servers_map.empty()) {
+ out << Substitute(kMsgDivergedFlags,
+ ServerTypeToString(ServerType::TABLET_SERVER),
+ ServerTypeToString(ServerType::MASTER)) << endl;
+ RETURN_NOT_OK(PrintFlagTable(ServerType::TABLET_SERVER,
+ cluster_status.tserver_summaries.size(),
+ tserver_diverged_flag_to_servers_map,
+ out));
out << endl;
}
}
@@ -234,8 +286,8 @@ Status KsckResults::PrintTo(PrintMode mode, int sections, ostream& out) {
// Finally, in the "server section", print the version summary.
if (sections & PrintSections::VERSION_SUMMARIES) {
RETURN_NOT_OK(PrintVersionTable(version_summaries,
- cluster_status.master_summaries.size()
- + cluster_status.tserver_summaries.size(),
+ cluster_status.master_summaries.size() +
+ cluster_status.tserver_summaries.size(),
out));
out << endl;
}
@@ -432,11 +484,29 @@ Status PrintServerHealthSummaries(ServerType type,
Status PrintFlagTable(ServerType type,
int num_servers,
const KsckFlagToServersMap& flag_to_servers_map,
- const KsckFlagTagsMap& flag_tags_map,
ostream& out) {
if (flag_to_servers_map.empty()) {
return Status::OK();
}
+ DataTable flags_table({"Flag", "Value", ServerTypeToString(type)});
+ for (const auto& flag : flag_to_servers_map) {
+ const string& name = flag.first.first;
+ const string& value = flag.first.second;
+ flags_table.AddRow({name,
+ value,
+ ServerCsv(num_servers, flag.second)});
+ }
+ return flags_table.PrintTo(out);
+}
+
+Status PrintTaggedFlagTable(ServerType type,
+ int num_servers,
+ const KsckFlagToServersMap& flag_to_servers_map,
+ const KsckFlagTagsMap& flag_tags_map,
+ ostream& out) {
+ if (flag_to_servers_map.empty()) {
+ return Status::OK();
+ }
DataTable flags_table({"Flag", "Value", "Tags", ServerTypeToString(type)});
for (const auto& flag : flag_to_servers_map) {
const string& name = flag.first.first;
diff --git a/src/kudu/tools/ksck_results.h b/src/kudu/tools/ksck_results.h
index ec69b6b..4d68086 100644
--- a/src/kudu/tools/ksck_results.h
+++ b/src/kudu/tools/ksck_results.h
@@ -126,10 +126,16 @@ struct KsckResults {
KsckVersionToServersMap version_summaries;
// Information about the flags of masters and tablet servers.
- KsckFlagToServersMap master_flag_to_servers_map;
- KsckFlagTagsMap master_flag_tags_map;
- KsckFlagToServersMap tserver_flag_to_servers_map;
- KsckFlagTagsMap tserver_flag_tags_map;
+ KsckFlagToServersMap master_checked_flag_to_servers_map;
+ KsckFlagToServersMap master_unusual_flag_to_servers_map;
+ KsckFlagTagsMap master_unusual_flag_tags_map;
+ KsckFlagToServersMap tserver_checked_flag_to_servers_map;
+ KsckFlagToServersMap tserver_unusual_flag_to_servers_map;
+ KsckFlagTagsMap tserver_unusual_flag_tags_map;
+
+ // Information on flags diverged between masters and tablet servers.
+ KsckFlagToServersMap master_diverged_flag_to_servers_map;
+ KsckFlagToServersMap tserver_diverged_flag_to_servers_map;
// Any special states that the tablet servers may be in.
KsckTServerStateMap ts_states;
@@ -156,15 +162,22 @@ Status PrintServerHealthSummaries(
const std::vector<cluster_summary::ServerHealthSummary>& summaries,
std::ostream& out);
-// Print a formatted summary of the flags in 'flag_to_servers_map', indicating
-// which servers have which (flag, value) pairs set.
-// Flag tag information is sourced from 'flag_tags_map'.
+// Print a formatted summary of the flags in 'flag_to_servers_map',
+// indicating which servers have which (flag, value) pairs set.
Status PrintFlagTable(cluster_summary::ServerType type,
int num_servers,
const KsckFlagToServersMap& flag_to_servers_map,
- const KsckFlagTagsMap& flag_tags_map,
std::ostream& out);
+// Similar to PrintFlagTable(), but also output information on tags for
+// flags in 'flag_to_servers_map'. Flag tag information is sourced from
+// 'flag_tags_map'.
+Status PrintTaggedFlagTable(cluster_summary::ServerType type,
+ int num_servers,
+ const KsckFlagToServersMap& flag_to_servers_map,
+ const KsckFlagTagsMap& flag_tags_map,
+ std::ostream& out);
+
Status PrintTServerStatesTable(const KsckTServerStateMap& ts_states,
std::ostream& out);
diff --git a/src/kudu/tools/tool_action_cluster.cc b/src/kudu/tools/tool_action_cluster.cc
index 4f8f4b9..81316b2 100644
--- a/src/kudu/tools/tool_action_cluster.cc
+++ b/src/kudu/tools/tool_action_cluster.cc
@@ -386,6 +386,7 @@ unique_ptr<Mode> BuildClusterMode() {
.AddOptionalParameter("color")
.AddOptionalParameter("consensus")
.AddOptionalParameter("fetch_info_concurrency")
+ .AddOptionalParameter("flags_categories_to_check")
.AddOptionalParameter("ksck_format")
.AddOptionalParameter("quiescing_info")
.AddOptionalParameter("sections")