kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [1/5] kudu git commit: [catalog_manager] categorization of rw operation failures
Date Thu, 11 May 2017 00:44:21 GMT
Repository: kudu
Updated Branches:
  refs/heads/master b927e80a8 -> e4e59bb5a


[catalog_manager] categorization of rw operation failures

This changelist introduces the categorization of the system catalog's
read and write operation failures which happen on leader post-election
callback. There are two categories of errors: fatal and non-fatal.

If an operation against the system catalog fails in between terms of
the catalog leadership, the error is considered non-fatal. In case of
a non-fatal error the leader post-election task bails out: the catalog
is no longer the leader at the original term and the task should be
executed by the new leader upon execution of the ElectedAsLeaderCb.

If an operation against the system catalog fails within the same term
of catalog leadership, the error is considered fatal and that causes the
master process to crash (with an exception of writing a newly generated
TSK when the TokenSigner still has a TSK to use). This is to avoid a
possible inconsistency when working with the tables/tablets metadata,
the IPKI certificate authority information and the Token Signing Keys.

Any failure of a read or write operation against the system catalog
happened during the catalog's shutdown is ignored and the leader
post-election task bails out once detecting such failure.

The same policy applies to other (i.e. not specific to read and write
operations against the system catalog) errors which might happen while
working with the IPKI certificate authority information and TokenSigner.
The rationale is the same as for handling the system catalog operation
failures: in case of an error, the leader has no consistent information
to work with, meanwhile a non-leader does not use the information
affected by the failure at all and can safely ignore the error.

Added a test to verify that the master server does not crash if change
of leadership detected while trying to persist a newly generated TSK
(Token Signing Key).

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Reviewed-on: http://gerrit.cloudera.org:8080/6170
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <davidralves@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/6b6593a0
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6b6593a0
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6b6593a0

Branch: refs/heads/master
Commit: 6b6593a0ce9dff6d4adf35084d3e4aba24816a26
Parents: b927e80
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Mon Feb 27 14:40:41 2017 -0800
Committer: David Ribeiro Alves <davidralves@gmail.com>
Committed: Wed May 10 22:46:27 2017 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/CMakeLists.txt       |   1 +
 .../catalog_manager_tsk-itest.cc                | 166 ++++++++
 src/kudu/master/catalog_manager.cc              | 395 +++++++++++++------
 src/kudu/master/catalog_manager.h               |  54 ++-
 src/kudu/master/master-test.cc                  |   2 +-
 src/kudu/master/master_service.cc               |  15 +-
 src/kudu/master/sys_catalog-test.cc             |   4 +-
 7 files changed, 497 insertions(+), 140 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/integration-tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index e7430b2..5cb1500 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -53,6 +53,7 @@ set(KUDU_TEST_LINK_LIBS integration-tests ${KUDU_MIN_TEST_LIBS})
 ADD_KUDU_TEST(all_types-itest RESOURCE_LOCK "master-rpc-ports")
 ADD_KUDU_TEST(alter_table-randomized-test)
 ADD_KUDU_TEST(alter_table-test)
+ADD_KUDU_TEST(catalog_manager_tsk-itest)
 ADD_KUDU_TEST(client_failover-itest)
 ADD_KUDU_TEST(client-stress-test
   RESOURCE_LOCK "master-rpc-ports"

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/integration-tests/catalog_manager_tsk-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/catalog_manager_tsk-itest.cc b/src/kudu/integration-tests/catalog_manager_tsk-itest.cc
new file mode 100644
index 0000000..60bc55d
--- /dev/null
+++ b/src/kudu/integration-tests/catalog_manager_tsk-itest.cc
@@ -0,0 +1,166 @@
+// 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.
+
+#include <algorithm>
+#include <iterator>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "kudu/client/client.h"
+#include "kudu/client/client-test-util.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/integration-tests/external_mini_cluster.h"
+#include "kudu/tablet/key_value_test_schema.h"
+#include "kudu/util/test_util.h"
+
+using kudu::client::KuduClient;
+using kudu::client::KuduClientBuilder;
+using kudu::client::KuduInsert;
+using kudu::client::KuduSchema;
+using kudu::client::KuduSession;
+using kudu::client::KuduTable;
+using kudu::client::KuduTableCreator;
+using std::back_inserter;
+using std::copy;
+using std::string;
+using std::unique_ptr;
+using std::vector;
+using strings::Substitute;
+
+namespace kudu {
+namespace master {
+
+class CatalogManagerTskITest : public KuduTest {
+ public:
+  CatalogManagerTskITest()
+      : num_masters_(3),
+        num_tservers_(1),
+#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
+        hb_interval_ms_(32),
+#else
+        hb_interval_ms_(16),
+#endif
+        run_time_seconds_(AllowSlowTests() ? 300 : 10) {
+
+    cluster_opts_.num_masters = num_masters_;
+    cluster_opts_.master_rpc_ports = { 11030, 11031, 11032 };
+    cluster_opts_.num_tablet_servers = num_tservers_;
+
+    // Add common flags for both masters and tservers.
+    const vector<string> common_flags = {
+      Substitute("--raft_heartbeat_interval_ms=$0", hb_interval_ms_),
+    };
+    copy(common_flags.begin(), common_flags.end(),
+        back_inserter(cluster_opts_.extra_master_flags));
+    copy(common_flags.begin(), common_flags.end(),
+        back_inserter(cluster_opts_.extra_tserver_flags));
+
+    // Add master-only flags.
+    const vector<string> master_flags = {
+      "--catalog_manager_inject_latency_prior_tsk_write_ms=1000",
+      "--raft_enable_pre_election=false",
+      Substitute("--leader_failure_exp_backoff_max_delta_ms=$0",
+          hb_interval_ms_ * 4),
+      "--leader_failure_max_missed_heartbeat_periods=1.0",
+      "--master_non_leader_masters_propagate_tsk",
+      "--tsk_rotation_seconds=2",
+      Substitute("--authn_token_validity_seconds=$0", run_time_seconds_),
+    };
+    copy(master_flags.begin(), master_flags.end(),
+        back_inserter(cluster_opts_.extra_master_flags));
+
+    // Add tserver-only flags.
+    const vector<string> tserver_flags = {
+      Substitute("--heartbeat_interval_ms=$0", hb_interval_ms_),
+    };
+    copy(tserver_flags.begin(), tserver_flags.end(),
+        back_inserter(cluster_opts_.extra_tserver_flags));
+  }
+
+  void StartCluster() {
+    cluster_.reset(new ExternalMiniCluster(cluster_opts_));
+    ASSERT_OK(cluster_->Start());
+  }
+
+  void SmokeTestCluster() {
+    using ::kudu::client::sp::shared_ptr;
+    static const char* kTableName = "test-table";
+    // Using the setting for both RPC and admin operation timeout.
+    const MonoDelta timeout = MonoDelta::FromSeconds(run_time_seconds_);
+    KuduClientBuilder builder;
+    builder.default_admin_operation_timeout(timeout).default_rpc_timeout(timeout);
+    shared_ptr<KuduClient> client;
+    ASSERT_OK(cluster_->CreateClient(&builder, &client));
+
+    // Create a table.
+    KuduSchema schema = client::KuduSchemaFromSchema(CreateKeyValueTestSchema());
+    gscoped_ptr<KuduTableCreator> table_creator(client->NewTableCreator());
+
+    ASSERT_OK(table_creator->table_name(kTableName)
+              .set_range_partition_columns({ "key" })
+              .schema(&schema)
+              .num_replicas(num_tservers_)
+              .Create());
+
+    // Insert a row.
+    shared_ptr<KuduTable> table;
+    ASSERT_OK(client->OpenTable(kTableName, &table));
+    unique_ptr<KuduInsert> ins(table->NewInsert());
+    ASSERT_OK(ins->mutable_row()->SetInt32(0, 12345));
+    ASSERT_OK(ins->mutable_row()->SetInt32(1, 54321));
+    shared_ptr<KuduSession> session = client->NewSession();
+    ASSERT_OK(session->Apply(ins.release()));
+    FlushSessionOrDie(session);
+
+    // Read it back.
+    ASSERT_EQ(1, CountTableRows(table.get()));
+
+    // Delete the table.
+    ASSERT_OK(client->DeleteTable(kTableName));
+  }
+
+ protected:
+  const int num_masters_;
+  const int num_tservers_;
+  const int hb_interval_ms_;
+  const int64_t run_time_seconds_;
+  ExternalMiniClusterOptions cluster_opts_;
+  std::shared_ptr<ExternalMiniCluster> cluster_;
+};
+
+// Check that master servers do not crash on change of leadership while
+// writing newly generated TSKs. The leadership changes are provoked
+// by the injected latency just after generating a TSK but prior to writing it
+// into the system table: setting --leader_failure_max_missed_heartbeat_periods
+// flag to just one heartbeat period and unsetting --raft_enable_pre_election
+// gives high chances of re-election to happen while current leader has blocked
+// its leadership-related activity.
+TEST_F(CatalogManagerTskITest, LeadershipChangeOnTskGeneration) {
+  NO_FATALS(StartCluster());
+
+  const MonoTime t_stop = MonoTime::Now() +
+      MonoDelta::FromSeconds(run_time_seconds_);
+  while (MonoTime::Now() < t_stop) {
+    NO_FATALS(SmokeTestCluster());
+  }
+
+  NO_FATALS(cluster_->AssertNoCrashes());
+}
+
+} // namespace master
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 56904ff..a6763a7 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -43,6 +43,7 @@
 
 #include <algorithm>
 #include <condition_variable>
+#include <functional>
 #include <memory>
 #include <mutex>
 #include <set>
@@ -89,6 +90,7 @@
 #include "kudu/security/token_signing_key.h"
 #include "kudu/tserver/tserver_admin.proxy.h"
 #include "kudu/util/debug/trace_event.h"
+#include "kudu/util/fault_injection.h"
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/monotime.h"
@@ -200,6 +202,13 @@ DEFINE_bool(catalog_manager_delete_orphaned_tablets, false,
             "master failures!");
 TAG_FLAG(catalog_manager_delete_orphaned_tablets, advanced);
 
+DEFINE_int32(catalog_manager_inject_latency_prior_tsk_write_ms, 0,
+             "Injects a random sleep between 0 and this many milliseconds "
+             "prior to writing newly generated TSK into the system table. "
+             "This is a test-only flag, do not use in production.");
+TAG_FLAG(catalog_manager_inject_latency_prior_tsk_write_ms, hidden);
+TAG_FLAG(catalog_manager_inject_latency_prior_tsk_write_ms, unsafe);
+
 using std::pair;
 using std::set;
 using std::shared_ptr;
@@ -452,14 +461,6 @@ void CatalogManagerBgTasks::Run() {
                        << l.catalog_status().ToString();
         }
       } else if (l.leader_status().ok()) {
-        // If this is the leader master, check if it's time to generate
-        // and store a new TSK (Token Signing Key).
-        Status s = catalog_manager_->CheckGenerateNewTskUnlocked();
-        if (!s.ok()) {
-          LOG(ERROR) << "Error processing TSK entry (will try next time): "
-                     << s.ToString();
-        }
-
         // Get list of tablets not yet running.
         std::vector<scoped_refptr<TabletInfo>> to_process;
         catalog_manager_->ExtractTabletsToProcess(&to_process);
@@ -474,8 +475,32 @@ void CatalogManagerBgTasks::Run() {
             //
             // TODO(unknown): Add tests for this in the revision that makes
             // create/alter fault tolerant.
-            LOG(ERROR) << "Error processing pending assignments, "
-                "aborting the current task: " << s.ToString();
+            LOG(ERROR) << "Error processing pending assignments: " << s.ToString();
+          }
+        }
+
+        // If this is the leader master, check if it's time to generate
+        // and store a new TSK (Token Signing Key).
+        Status s = catalog_manager_->TryGenerateNewTskUnlocked();
+        if (!s.ok()) {
+          const TokenSigner* signer = catalog_manager_->master_->token_signer();
+          const string err_msg = "failed to refresh TSK: " + s.ToString() + ": ";
+          if (l.has_term_changed()) {
+            LOG(INFO) << err_msg
+                      << "ignoring the error since not the leader anymore";
+          } else if (signer->IsCurrentKeyValid()) {
+            LOG(WARNING) << err_msg << "will try again next cycle";
+          } else {
+            // The TokenSigner ended up with no valid key to use. If the catalog
+            // manager is still the leader, it would not be able to create valid
+            // authn token signatures. It's not clear how to properly resolve
+            // this situation and keep the process running. To avoid possible
+            // inconsistency, let's crash the process.
+            //
+            // NOTE: This can only happen in a multi-master Kudu cluster. In
+            //       that case, after this particular master crashes, another
+            //       master will take over as leader.
+            LOG(FATAL) << err_msg;
           }
         }
       }
@@ -715,7 +740,7 @@ Status CatalogManager::Init(bool is_first_run) {
 
 Status CatalogManager::ElectedAsLeaderCb() {
   return leader_election_pool_->SubmitClosure(
-      Bind(&CatalogManager::VisitTablesAndTabletsTask, Unretained(this)));
+      Bind(&CatalogManager::PrepareForLeadershipTask, Unretained(this)));
 }
 
 Status CatalogManager::WaitUntilCaughtUpAsLeader(const MonoDelta& timeout) {
@@ -733,32 +758,104 @@ Status CatalogManager::WaitUntilCaughtUpAsLeader(const MonoDelta& timeout) {
   return Status::OK();
 }
 
-Status CatalogManager::LoadCertAuthorityInfo(unique_ptr<PrivateKey>* key,
-                                             unique_ptr<Cert>* cert) {
+Status CatalogManager::InitCertAuthority() {
   leader_lock_.AssertAcquiredForWriting();
 
-  SysCertAuthorityEntryPB info;
-  RETURN_NOT_OK(sys_catalog_->GetCertAuthorityEntry(&info));
-
-  unique_ptr<PrivateKey> ca_private_key(new PrivateKey);
-  unique_ptr<Cert> ca_cert(new Cert);
-  RETURN_NOT_OK(ca_private_key->FromString(
-      info.private_key(), DataFormat::DER));
-  RETURN_NOT_OK(ca_cert->FromString(
-      info.certificate(), DataFormat::DER));
-  // Extra sanity check.
-  RETURN_NOT_OK(ca_cert->CheckKeyMatch(*ca_private_key));
-
-  key->swap(ca_private_key);
-  cert->swap(ca_cert);
+  unique_ptr<PrivateKey> key;
+  unique_ptr<Cert> cert;
+  const Status s = LoadCertAuthorityInfo(&key, &cert);
+  if (s.ok()) {
+    return InitCertAuthorityWith(std::move(key), std::move(cert));
+  }
+  if (s.IsNotFound()) {
+    // Status::NotFound is returned if no IPKI certificate authority record is
+    // found in the system catalog table. It can happen on the very first run
+    // of a secured Kudu cluster. If so, it's necessary to create and persist
+    // a new CA record which, if persisted, will be used for this and next runs.
+    //
+    // The subtlety here is that first it's necessary to store the newly
+    // generated IPKI CA information (the private key and the certificate) into
+    // the system table and only after that initialize the master certificate
+    // authority. This protects against a leadership change between the
+    // generation and the usage of the newly generated IPKI CA information
+    // by the master.
+    //
+    // An example of such 'leadership change in the middle' scenario:
+    //
+    // 1. The catalog manager starts generating Kudu  IPKI CA private key and
+    //    corresponding certificate. This takes some time since generating
+    //    a cryptographically strong private key requires many CPU cycles.
+    //
+    // 2. While the catalog manager is busy with generating the CA info, a new
+    //    election happens in the background and the catalog manager loses its
+    //    leadership role.
+    //
+    // 3. The catalog manager tries to write the newly generated information
+    //    into the system table. There are two possible cases at the time when
+    //    applying the write operation:
+    //
+    //      a. The catalog manager is not the system tablet's leader.
+    //
+    //      b. The catalog manager is the system tablet's leader.
+    //         It regained its leadership role by the time the write operation
+    //         is applied. That can happen if another election occurs before
+    //         the write operation is applied.
+    //
+    // 4. Essentially, the following responses are possible for the write
+    //    operation, enumerated in accordance with 3.{a,b} items above:
+    //
+    //      a. A failure happens and corresponding error message is logged;
+    //         the failure is ignored.
+    //
+    //      b. In the case when the catalog manager becomes the leader again,
+    //         there are two possible outcomes for the write operation:
+    //
+    //           i.  Success. The master completes the initialization process
+    //               and proceeds to serve client requests.
+    //
+    //           ii. Failure. This is when the former in-the-middle leader has
+    //               succeeded in writing its CA info into the system table.
+    //               That could happen if the former in-the-middle leader was
+    //               very fast because there were plenty of CPU resources
+    //               available for CA info generation. Since the CA info record
+    //               has pre-defined identifier, it's impossible to have more
+    //               than one CA info record in the system table. This is due to
+    //               the {record_id, record_type} uniqueness constraint.
+    //
+    // In case of the write operation's success (4.b.i), it's safe to proceed
+    // with loading the persisted CA information into the CertAuthority run-time
+    // object.
+    //
+    // In case of the write operation's failure (4.a, 4.b.ii), the generated
+    // CA information is no longer relevant and can be safely discarded. The
+    // crucial point is to not initialize the CertAuthority with non-persisted
+    // information. Otherwise that information could get into the run-time
+    // structures of some system components, cutting them off from communicating
+    // with the rest of the system which uses the genuine CA information.
+    //
+    // Once the CA information is persisted in the system table, a catalog
+    // manager reads and loads it into the CertAuthority every time it becomes
+    // an elected leader.
+    unique_ptr<PrivateKey> key(new PrivateKey);
+    unique_ptr<Cert> cert(new Cert);
+
+    // Generate new private key and corresponding CA certificate.
+    RETURN_NOT_OK(master_->cert_authority()->Generate(key.get(), cert.get()));
+    // If the leadership was lost, writing into the system table fails.
+    RETURN_NOT_OK(StoreCertAuthorityInfo(*key, *cert));
+    // Once the CA information is persisted, it's necessary to initialize
+    // the certificate authority sub-component with it. The leader master
+    // should not run without a CA certificate.
+    return InitCertAuthorityWith(std::move(key), std::move(cert));
+  }
 
-  return Status::OK();
+  return s;
 }
 
 // Initialize the master's certificate authority component with the specified
 // private key and certificate.
-Status CatalogManager::InitCertAuthority(unique_ptr<PrivateKey> key,
-                                         unique_ptr<Cert> cert) {
+Status CatalogManager::InitCertAuthorityWith(
+    unique_ptr<PrivateKey> key, unique_ptr<Cert> cert) {
   leader_lock_.AssertAcquiredForWriting();
   auto* ca = master_->cert_authority();
   RETURN_NOT_OK_PREPEND(ca->Init(std::move(key), std::move(cert)),
@@ -780,6 +877,28 @@ Status CatalogManager::InitCertAuthority(unique_ptr<PrivateKey> key,
   return Status::OK();
 }
 
+Status CatalogManager::LoadCertAuthorityInfo(unique_ptr<PrivateKey>* key,
+                                             unique_ptr<Cert>* cert) {
+  leader_lock_.AssertAcquiredForWriting();
+
+  SysCertAuthorityEntryPB info;
+  RETURN_NOT_OK(sys_catalog_->GetCertAuthorityEntry(&info));
+
+  unique_ptr<PrivateKey> ca_private_key(new PrivateKey);
+  unique_ptr<Cert> ca_cert(new Cert);
+  RETURN_NOT_OK(ca_private_key->FromString(
+      info.private_key(), DataFormat::DER));
+  RETURN_NOT_OK(ca_cert->FromString(
+      info.certificate(), DataFormat::DER));
+  // Extra sanity check.
+  RETURN_NOT_OK(ca_cert->CheckKeyMatch(*ca_private_key));
+
+  key->swap(ca_private_key);
+  cert->swap(ca_cert);
+
+  return Status::OK();
+}
+
 // Store internal Kudu CA cert authority information into the system table.
 Status CatalogManager::StoreCertAuthorityInfo(const PrivateKey& key,
                                               const Cert& cert) {
@@ -789,28 +908,37 @@ Status CatalogManager::StoreCertAuthorityInfo(const PrivateKey& key,
   RETURN_NOT_OK(key.ToString(info.mutable_private_key(), DataFormat::DER));
   RETURN_NOT_OK(cert.ToString(info.mutable_certificate(), DataFormat::DER));
   RETURN_NOT_OK(sys_catalog_->AddCertAuthorityEntry(info));
-  LOG(INFO) << "Successfully stored the newly generated cert authority "
-               "information into the system table.";
+  LOG(INFO) << "Generated new certificate authority record";
 
   return Status::OK();
 }
 
-void CatalogManager::VisitTablesAndTabletsTask() {
+Status CatalogManager::InitTokenSigner() {
+  leader_lock_.AssertAcquiredForWriting();
+
+  set<string> expired_tsk_entry_ids;
+  RETURN_NOT_OK(LoadTskEntries(&expired_tsk_entry_ids));
+  RETURN_NOT_OK(TryGenerateNewTskUnlocked());
+  return DeleteTskEntries(expired_tsk_entry_ids);
+}
+
+void CatalogManager::PrepareForLeadershipTask() {
   {
     // Hack to block this function until InitSysCatalogAsync() is finished.
     shared_lock<LockType> l(lock_);
   }
   const Consensus* consensus = sys_catalog_->tablet_replica()->consensus();
-  int64_t term = consensus->ConsensusState(CONSENSUS_CONFIG_COMMITTED).current_term();
+  const int64_t term_before_wait =
+      consensus->ConsensusState(CONSENSUS_CONFIG_COMMITTED).current_term();
   {
     std::lock_guard<simple_spinlock> l(state_lock_);
-    if (leader_ready_term_ == term) {
+    if (leader_ready_term_ == term_before_wait) {
       // The term hasn't changed since the last time this master was the
       // leader. It's not possible for another master to be leader for the same
       // term, so there hasn't been any actual leadership change and thus
       // there's no reason to reload the on-disk metadata.
       VLOG(2) << Substitute("Term $0 hasn't changed, ignoring dirty callback",
-                            term);
+                            term_before_wait);
       return;
     }
   }
@@ -826,86 +954,91 @@ void CatalogManager::VisitTablesAndTabletsTask() {
     return;
   }
 
-  int64_t term_after_wait = consensus->ConsensusState(CONSENSUS_CONFIG_COMMITTED).current_term();
-  if (term_after_wait != term) {
+  const int64_t term =
+      consensus->ConsensusState(CONSENSUS_CONFIG_COMMITTED).current_term();
+  if (term_before_wait != term) {
     // If we got elected leader again while waiting to catch up then we will
     // get another callback to visit the tables and tablets, so bail.
-    LOG(INFO) << "Term change from " << term << " to " << term_after_wait
+    LOG(INFO) << "Term changed from " << term_before_wait << " to " << term
         << " while waiting for master leader catchup. Not loading sys catalog metadata";
     return;
   }
 
   {
+    // This lambda returns the result of calling the 'func', checking whether
+    // the error, if any, is fatal for the leader catalog. If the returned
+    // status is non-OK, the caller should bail on the leadership preparation
+    // task. If the error is considered fatal, LOG(FATAL) is called.
+    const auto check = [this](
+        std::function<Status()> func,
+        const Consensus& consensus,
+        int64_t start_term,
+        const char* op_description) {
+
+      leader_lock_.AssertAcquiredForWriting();
+      const Status s = func();
+      if (s.ok()) {
+        // Not an error at all.
+        return s;
+      }
+
+      {
+        std::lock_guard<simple_spinlock> l(state_lock_);
+        if (state_ == kClosing) {
+          // Errors on shutdown are not considered fatal.
+          LOG(INFO) << op_description
+                    << " failed due to the shutdown of the catalog: "
+                    << s.ToString();
+          return s;
+        }
+      }
+
+      const int64_t term =
+          consensus.ConsensusState(CONSENSUS_CONFIG_COMMITTED).current_term();
+      if (term != start_term) {
+        // If the term has changed we assume the new leader catalog is about
+        // to do the necessary work in its leadership preparation task.
+        LOG(INFO) << op_description << " failed; "
+                  << Substitute("change in term detected: $0 vs $1: ",
+                                start_term, term)
+                  << s.ToString();
+        return s;
+      }
+
+      // In all other cases non-OK status is considered fatal.
+      LOG(FATAL) << op_description << " failed: " << s.ToString();
+      return s; // unreachable
+    };
+
     // Block new catalog operations, and wait for existing operations to finish.
     std::lock_guard<RWMutex> leader_lock_guard(leader_lock_);
 
-    LOG(INFO) << "Loading table and tablet metadata into memory...";
-    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() +
-                       "Loading metadata into memory") {
-      CHECK_OK(VisitTablesAndTabletsUnlocked());
-    }
-
-    // TODO(KUDU-1920): this should not be done in case of external PKI.
-    // TODO(KUDU-1919): some kind of tool to rotate the IPKI CA
-    LOG(INFO) << "Loading CA info into memory...";
-    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() +
-                       "Loading CA info into memory") {
-      unique_ptr<PrivateKey> key;
-      unique_ptr<Cert> cert;
-      const Status& s = LoadCertAuthorityInfo(&key, &cert);
-      if (s.ok()) {
-        // Once succesfully loaded, the CA information is supposed to be valid:
-        // the leader master should not be run without CA certificate.
-        CHECK_OK(InitCertAuthority(std::move(key), std::move(cert)));
-      } else if (s.IsNotFound()) {
-        LOG(INFO) << "Did not find CA certificate and key for Kudu IPKI, "
-                     "will generate new ones";
-        // No CA information record has been found in the table -- generate
-        // a new one. The subtlety here is that first it's necessary to store
-        // the newly generated information into the system table and only after
-        // that initialize master certificate authority. The reason is:
-        // if the master server loses its leadership role by that time, there
-        // will be an error on an attempt to write into the system table.
-        // If that happens, skip the rest of the sequence: when this callback
-        // is invoked next time, the system table should already contain
-        // CA certificate information written by other master.
-        unique_ptr<PrivateKey> private_key(new PrivateKey);
-        unique_ptr<Cert> cert(new Cert);
-
-        // Generate new private key and corresponding CA certificate.
-        CHECK_OK(master_->cert_authority()->Generate(private_key.get(),
-                                                     cert.get()));
-        // It the leadership role is lost at this moment, writing into the
-        // system table will fail.
-        const Status& s = StoreCertAuthorityInfo(*private_key, *cert);
-        if (s.ok()) {
-          // The leader master should not run without CA certificate.
-          CHECK_OK(InitCertAuthority(std::move(private_key), std::move(cert)));
-        } else {
-          LOG(WARNING) << "Failed to write newly generated CA information into "
-                          "the system table, assuming change of leadership: "
-                       << s.ToString();
-        }
-      } else {
-        CHECK_OK(s);
+    static const char* const kLoadMetaOpDescription =
+        "Loading table and tablet metadata into memory";
+    LOG(INFO) << kLoadMetaOpDescription << "...";
+    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() + kLoadMetaOpDescription) {
+      if (!check(std::bind(&CatalogManager::VisitTablesAndTabletsUnlocked, this),
+                 *consensus, term, kLoadMetaOpDescription).ok()) {
+        return;
       }
     }
 
-    LOG(INFO) << "Loading token signing keys...";
-    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() +
-                       "Loading token signing keys...") {
-      set<string> expired_tsk_entry_ids;
-      CHECK_OK(LoadTskEntries(&expired_tsk_entry_ids));
-      Status s = CheckGenerateNewTskUnlocked();
-      if (!s.ok()) {
-        LOG(WARNING) << "Failed to generate and persist new TSK, "
-                        "assuming change of leadership: " << s.ToString();
+    // TODO(KUDU-1920): update this once "BYO PKI" feature is supported.
+    static const char* const kCaInitOpDescription =
+        "Initializing Kudu internal certificate authority";
+    LOG(INFO) << kCaInitOpDescription << "...";
+    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() + kCaInitOpDescription) {
+      if (!check(std::bind(&CatalogManager::InitCertAuthority, this),
+                 *consensus, term, kCaInitOpDescription).ok()) {
         return;
       }
-      s = DeleteTskEntries(expired_tsk_entry_ids);
-      if (!s.ok()) {
-        LOG(WARNING) << "Failed to purge expired TSK entries from system table, "
-                        "assuming change of leadership: " << s.ToString();
+    }
+
+    static const char* const kTskOpDescription = "Loading token signing keys";
+    LOG(INFO) << kTskOpDescription << "...";
+    LOG_SLOW_EXECUTION(WARNING, 1000, LogPrefix() + kTskOpDescription) {
+      if (!check(std::bind(&CatalogManager::InitTokenSigner, this),
+                 *consensus, term, kTskOpDescription).ok()) {
         return;
       }
     }
@@ -3279,8 +3412,9 @@ void CatalogManager::SendDeleteTabletRequest(const scoped_refptr<TabletInfo>& ta
     return;
   }
   const ConsensusStatePB& cstate = tablet_lock.data().pb.committed_consensus_state();
-  LOG(INFO) << "Sending DeleteTablet for " << cstate.config().peers().size()
-            << " replicas of tablet " << tablet->tablet_id();
+  LOG_WITH_PREFIX(INFO)
+      << "Sending DeleteTablet for " << cstate.config().peers().size()
+      << " replicas of tablet " << tablet->tablet_id();
   for (const auto& peer : cstate.config().peers()) {
     SendDeleteReplicaRequest(tablet->tablet_id(), TABLET_DATA_DELETED,
                              boost::none, tablet->table(),
@@ -3318,6 +3452,8 @@ void CatalogManager::SendAddServerRequest(const scoped_refptr<TabletInfo>& table
               Substitute("Failed to send AddServer request for tablet $0", tablet->tablet_id()));
   // We can't access 'task' after calling Run() because it may delete itself
   // inside Run() in the case that the tablet has no known leader.
+  LOG_WITH_PREFIX(INFO)
+      << "Started AddServer task for tablet " << tablet->tablet_id();
 }
 
 void CatalogManager::ExtractTabletsToProcess(
@@ -3359,7 +3495,7 @@ void CatalogManager::ExtractTabletsToProcess(
 //   2) Try to write it to the system table.
 //   3) Pass it back to the TokenSigner on success.
 //   4) Check and switch TokenSigner to the new key if it's time to do so.
-Status CatalogManager::CheckGenerateNewTskUnlocked() {
+Status CatalogManager::TryGenerateNewTskUnlocked() {
   TokenSigner* signer = master_->token_signer();
   unique_ptr<security::TokenSigningPrivateKey> tsk;
   RETURN_NOT_OK(signer->CheckNeedKey(&tsk));
@@ -3369,9 +3505,10 @@ Status CatalogManager::CheckGenerateNewTskUnlocked() {
     tsk->ExportPB(&tsk_pb);
     SysTskEntryPB sys_entry;
     sys_entry.mutable_tsk()->Swap(&tsk_pb);
+    MAYBE_INJECT_RANDOM_LATENCY(
+        FLAGS_catalog_manager_inject_latency_prior_tsk_write_ms);
     RETURN_NOT_OK(sys_catalog_->AddTskEntry(sys_entry));
-    LOG(INFO) << "Saved newly generated TSK " << tsk->key_seq_num()
-              << " into the system table.";
+    LOG_WITH_PREFIX(INFO) << "Generated new TSK " << tsk->key_seq_num();
     // Then add the new TSK into the signer.
     RETURN_NOT_OK(signer->AddKey(std::move(tsk)));
   }
@@ -3381,6 +3518,9 @@ Status CatalogManager::CheckGenerateNewTskUnlocked() {
 Status CatalogManager::LoadTskEntries(set<string>* expired_entry_ids) {
   TskEntryLoader loader;
   RETURN_NOT_OK(sys_catalog_->VisitTskEntries(&loader));
+  for (const auto& key : loader.entries()) {
+    LOG_WITH_PREFIX(INFO) << "Loaded TSK: " << key.key_seq_num();
+  }
   if (expired_entry_ids) {
     set<string> ref(loader.expired_entry_ids());
     expired_entry_ids->swap(ref);
@@ -3431,9 +3571,10 @@ void CatalogManager::HandleAssignCreatingTablet(TabletInfo* tablet,
   // within the timeout. So the tablet will be replaced by a new one.
   scoped_refptr<TabletInfo> replacement = CreateTabletInfo(tablet->table().get(),
                                                            old_info.pb.partition());
-  LOG(WARNING) << "Tablet " << tablet->ToString() << " was not created within "
-               << "the allowed timeout. Replacing with a new tablet "
-               << replacement->tablet_id();
+  LOG_WITH_PREFIX(WARNING)
+      << "Tablet " << tablet->ToString() << " was not created within "
+      << "the allowed timeout. Replacing with a new tablet "
+      << replacement->tablet_id();
 
   // Mark old tablet as replaced.
   tablet->mutable_metadata()->mutable_dirty()->set_state(
@@ -3484,12 +3625,15 @@ Status CatalogManager::HandleTabletSchemaVersionReport(TabletInfo *tablet, uint3
   actions.table_to_update = table;
   Status s = sys_catalog_->Write(actions);
   if (!s.ok()) {
-    LOG(WARNING) << "An error occurred while updating sys-tables: " << s.ToString();
+    LOG_WITH_PREFIX(WARNING)
+        << "An error occurred while updating sys-tables: " << s.ToString();
     return s;
   }
 
   l.Commit();
-  LOG(INFO) << table->ToString() << " - Alter table completed version=" << current_version;
+  LOG_WITH_PREFIX(INFO)
+      << table->ToString() << " - Alter table completed version="
+      << current_version;
   return Status::OK();
 }
 
@@ -3572,7 +3716,8 @@ Status CatalogManager::ProcessPendingAssignments(
   }
 
   if (!s.ok()) {
-    LOG(WARNING) << "Aborting the current task due to error: " << s.ToString();
+    LOG_WITH_PREFIX(WARNING)
+        << "Aborting the current task due to error: " << s.ToString();
     // If there was an error, abort any mutations started by the
     // current task.
     unlocker_out.Abort();
@@ -3882,7 +4027,9 @@ Status CatalogManager::GetTableLocations(const GetTableLocationsRequestPB* req,
       StatusToPB(s, resp->mutable_error()->mutable_status());
       break;
     } else {
-      LOG(FATAL) << "Unexpected error while building tablet locations: " << s.ToString();
+      LOG_WITH_PREFIX(FATAL)
+          << "Unexpected error while building tablet locations: "
+          << s.ToString();
     }
   }
   resp->set_ttl_millis(FLAGS_table_locations_ttl_ms);
@@ -3968,7 +4115,6 @@ void CatalogManager::AbortAndWaitForAllTasks(
     t->WaitTasksCompletion();
   }
 }
-
 ////////////////////////////////////////////////////////////
 // CatalogManager::ScopedLeaderSharedLock
 ////////////////////////////////////////////////////////////
@@ -3976,7 +4122,10 @@ void CatalogManager::AbortAndWaitForAllTasks(
 CatalogManager::ScopedLeaderSharedLock::ScopedLeaderSharedLock(
     CatalogManager* catalog)
     : catalog_(DCHECK_NOTNULL(catalog)),
-      leader_shared_lock_(catalog->leader_lock_, std::try_to_lock) {
+      leader_shared_lock_(catalog->leader_lock_, std::try_to_lock),
+      catalog_status_(Status::Uninitialized("")),
+      leader_status_(Status::Uninitialized("")),
+      initial_term_(-1) {
 
   // Check if the catalog manager is running.
   std::lock_guard<simple_spinlock> l(catalog_->state_lock_);
@@ -3986,11 +4135,13 @@ CatalogManager::ScopedLeaderSharedLock::ScopedLeaderSharedLock(
                    catalog_->state_));
     return;
   }
+  catalog_status_ = Status::OK();
 
   // Check if the catalog manager is the leader.
-  ConsensusStatePB cstate = catalog_->sys_catalog_->tablet_replica()->consensus()->
-      ConsensusState(CONSENSUS_CONFIG_COMMITTED);
-  string uuid = catalog_->master_->fs_manager()->uuid();
+  const ConsensusStatePB cstate = catalog_->sys_catalog_->tablet_replica()->
+      consensus()->ConsensusState(CONSENSUS_CONFIG_COMMITTED);
+  initial_term_ = cstate.current_term();
+  const string& uuid = catalog_->master_->fs_manager()->uuid();
   if (PREDICT_FALSE(!cstate.has_leader_uuid() || cstate.leader_uuid() != uuid)) {
     leader_status_ = Status::IllegalState(
         Substitute("Not the leader. Local UUID: $0, Consensus state: $1",
@@ -4003,6 +4154,14 @@ CatalogManager::ScopedLeaderSharedLock::ScopedLeaderSharedLock(
         "Leader not yet ready to serve requests");
     return;
   }
+  leader_status_ = Status::OK();
+}
+
+bool CatalogManager::ScopedLeaderSharedLock::has_term_changed() const {
+  DCHECK(leader_status().ok());
+  const ConsensusStatePB cstate = catalog_->sys_catalog_->tablet_replica()->
+      consensus()->ConsensusState(CONSENSUS_CONFIG_COMMITTED);
+  return cstate.current_term() != initial_term_;
 }
 
 template<typename RespClass>

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index 59ffe16..c28f15b 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -17,6 +17,7 @@
 #ifndef KUDU_MASTER_CATALOG_MANAGER_H
 #define KUDU_MASTER_CATALOG_MANAGER_H
 
+#include <functional>
 #include <map>
 #include <memory>
 #include <set>
@@ -354,6 +355,11 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
       return leader_status_;
     }
 
+    // Check whether the consensus configuration term has changed from the term
+    // captured at object construction (initial_term_).
+    // Requires: leader_status() returns OK().
+    bool has_term_changed() const;
+
     // Check that the catalog manager is initialized. It may or may not be the
     // leader of its Raft configuration.
     //
@@ -376,6 +382,7 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
     shared_lock<RWMutex> leader_shared_lock_;
     Status catalog_status_;
     Status leader_status_;
+    int64_t initial_term_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedLeaderSharedLock);
   };
@@ -541,8 +548,8 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   typedef std::unordered_map<std::string, scoped_refptr<TabletInfo>> TabletInfoMap;
 
   // Called by SysCatalog::SysCatalogStateChanged when this node
-  // becomes the leader of a consensus configuration. Executes VisitTablesAndTabletsTask
-  // via 'worker_pool_'.
+  // becomes the leader of a consensus configuration. Executes
+  // PrepareForLeadershipTask() via 'worker_pool_'.
   Status ElectedAsLeaderCb();
 
   // Loops and sleeps until one of the following conditions occurs:
@@ -557,9 +564,10 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   // reading that data, to ensure consistency across failovers.
   Status WaitUntilCaughtUpAsLeader(const MonoDelta& timeout);
 
-  // Performs several checks before calling VisitTablesAndTablets to actually
-  // reload table/tablet metadata into memory.
-  void VisitTablesAndTabletsTask();
+  // Performs several checks before calling VisitTablesAndTablets() to actually
+  // reload table/tablet metadata into memory and do other work to update the
+  // internal state of this object upon becoming the leader.
+  void PrepareForLeadershipTask();
 
   // Clears out the existing metadata ('table_names_map_', 'table_ids_map_',
   // and 'tablet_map_'), loads tables metadata into memory and if successful
@@ -576,21 +584,36 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   // This method is thread-safe.
   Status InitSysCatalogAsync(bool is_first_run);
 
-  // Load the internal Kudu certficate authority information from the system
+  // Initialize the IPKI certificate authority: load the CA information record
+  // from the system table. If the CA information record is not present in the
+  // table, generate and store a new one.
+  Status InitCertAuthority();
+
+  // Initialize the IPKI certificate authority with the specified private key
+  // and certificate.
+  Status InitCertAuthorityWith(std::unique_ptr<security::PrivateKey> key,
+                               std::unique_ptr<security::Cert> cert);
+
+  // Load the IPKI certficate authority information from the system
   // table: the private key and the certificate. If the CA info entry is not
   // found in the table, return Status::NotFound.
   Status LoadCertAuthorityInfo(std::unique_ptr<security::PrivateKey>* key,
                                std::unique_ptr<security::Cert>* cert);
 
-  // Initialize master's certificate authority with the specified private key
-  // and certificate.
-  Status InitCertAuthority(std::unique_ptr<security::PrivateKey> key,
-                           std::unique_ptr<security::Cert> cert);
-
-  // Store CA certificate information into the system table.
+  // Store the IPKI certificate authority information into the system table.
   Status StoreCertAuthorityInfo(const security::PrivateKey& key,
                                 const security::Cert& cert);
 
+  // 1. Initialize the TokenSigner (the component which signs authn tokens):
+  //      a. Load TSK records from the system table.
+  //      b. Import the newly loaded TSK records into the TokenSigner.
+  // 2. Check whether it's time to generate a new token signing key.
+  //    If yes, then:
+  //      a. Generate a new TSK.
+  //      b. Store the new TSK one into the system catalog table.
+  // 3. Purge expired TSKs from the system table.
+  Status InitTokenSigner();
+
   // Helper for creating the initial TableInfo state
   // Leaves the table "write locked" with the new info in the
   // "dirty" state field.
@@ -627,9 +650,10 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   // Extract the set of tablets that must be processed because not running yet.
   void ExtractTabletsToProcess(std::vector<scoped_refptr<TabletInfo>>* tablets_to_process);
 
-  // Check if it's time to generate new Token Signing Key for TokenSigner.
-  // If so, generate one and persist it into the system table.
-  Status CheckGenerateNewTskUnlocked();
+  // Check if it's time to generate a new Token Signing Key for TokenSigner.
+  // If so, generate one and persist it into the system table. After that,
+  // push it into the TokenSigner's key queue.
+  Status TryGenerateNewTskUnlocked();
 
   // Load non-expired TSK entries from the system table.
   // Once done, initialize TokenSigner with the loaded entries.

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/master/master-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 143797e..bc3aa99 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -702,7 +702,7 @@ TEST_F(MasterTest, TestShutdownDuringTableVisit) {
   ASSERT_OK(master_->catalog_manager()->ElectedAsLeaderCb());
 
   // Master will now shut down, potentially racing with
-  // CatalogManager::VisitTablesAndTabletsTask.
+  // CatalogManager::PrepareForLeadershipTask().
 }
 
 // Tests that the catalog manager handles spurious calls to ElectedAsLeaderCb()

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/master/master_service.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc
index 523f803..6476516 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -50,6 +50,12 @@ DEFINE_bool(master_support_connect_to_master_rpc, true,
 TAG_FLAG(master_support_connect_to_master_rpc, unsafe);
 TAG_FLAG(master_support_connect_to_master_rpc, hidden);
 
+DEFINE_bool(master_non_leader_masters_propagate_tsk, false,
+            "Whether a non-leader master sends information about its TSKs in "
+            "response to a tablet server's heartbeat. This is intended for "
+            "tests scenarios only and should not be used elsewhere.");
+TAG_FLAG(master_non_leader_masters_propagate_tsk, hidden);
+
 using kudu::security::SignedTokenPB;
 using google::protobuf::Message;
 using std::string;
@@ -191,9 +197,12 @@ void MasterServiceImpl::TSHeartbeat(const TSHeartbeatRequestPB* req,
     resp->add_ca_cert_der(server_->cert_authority()->ca_cert_der());
   }
 
-  // 7. Only leaders send public parts of non-expired TSK
-  // which the TS doesn't have.
-  if (is_leader_master && req->has_latest_tsk_seq_num()) {
+  // 7. Only leaders send public parts of non-expired TSK which the TS doesn't
+  //    have, except if the '--master_non_leader_masters_propagate_tsk'
+  //    test-only flag is set.
+  if ((is_leader_master ||
+       PREDICT_FALSE(FLAGS_master_non_leader_masters_propagate_tsk)) &&
+      req->has_latest_tsk_seq_num()) {
     auto tsk_public_keys = server_->token_signer()->verifier().ExportKeys(
         req->latest_tsk_seq_num());
     for (auto& key : tsk_public_keys) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6b6593a0/src/kudu/master/sys_catalog-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/sys_catalog-test.cc b/src/kudu/master/sys_catalog-test.cc
index 7d8f361..12d9c69 100644
--- a/src/kudu/master/sys_catalog-test.cc
+++ b/src/kudu/master/sys_catalog-test.cc
@@ -411,8 +411,6 @@ TEST_F(SysCatalogTest, LoadCertAuthorityInfo) {
 
 // Check that if the certificate authority information is already present,
 // it cannot be overwritten using SysCatalogTable::AddCertAuthorityInfo().
-// Basically, this is to verify that SysCatalogTable::AddCertAuthorityInfo()
-// can be called just once to store CA information on first cluster startup.
 TEST_F(SysCatalogTest, AttemptOverwriteCertAuthorityInfo) {
   // The system catalog should already contain newly generated CA private key
   // and certificate: the SetUp() method awaits for the catalog manager
@@ -422,7 +420,7 @@ TEST_F(SysCatalogTest, AttemptOverwriteCertAuthorityInfo) {
   ASSERT_OK(master_->catalog_manager()->sys_catalog()->
             GetCertAuthorityEntry(&ca_entry));
   const Status s = master_->catalog_manager()->sys_catalog()->
-            AddCertAuthorityEntry(ca_entry);
+      AddCertAuthorityEntry(ca_entry);
   ASSERT_TRUE(s.IsCorruption()) << s.ToString();
   ASSERT_EQ("Corruption: One or more rows failed to write", s.ToString());
 }


Mime
View raw message