[catalog_manager] cleanup on InitTokenSigner

With this patch, the catalog manager no longer issues empty write operations to the system tablet upon initializing TokenSigner. Additionally, the catalog manager now logs about deleted TSK keys. Change-Id: I68eb1972c0ab330c7739e6901ebe36e4190144cc Reviewed-on: http://gerrit.cloudera.org:8080/11902 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong <[email protected]>
acelyc111 · Nov 7, 2018 · 17061dd · 17061dd
1 parent ecdeac7
commit 17061dd
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 7 deletions.
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
@@ -939,7 +939,10 @@ Status CatalogManager::InitTokenSigner() {
   set<string> expired_tsk_entry_ids;
   RETURN_NOT_OK(LoadTskEntries(&expired_tsk_entry_ids));
   RETURN_NOT_OK(TryGenerateNewTskUnlocked());
-  return DeleteTskEntries(expired_tsk_entry_ids);
+  if (!expired_tsk_entry_ids.empty()) {
+    return DeleteTskEntries(expired_tsk_entry_ids);
+  }
+  return Status::OK();
 }
 
 void CatalogManager::PrepareForLeadershipTask() {
@@ -4116,7 +4119,10 @@ Status CatalogManager::LoadTspkEntries(vector<TokenSigningPublicKeyPB>* keys) {
 
 Status CatalogManager::DeleteTskEntries(const set<string>& entry_ids) {
   leader_lock_.AssertAcquiredForWriting();
-  return sys_catalog_->RemoveTskEntries(entry_ids);
+  RETURN_NOT_OK(sys_catalog_->RemoveTskEntries(entry_ids));
+  LOG_WITH_PREFIX(INFO) << Substitute("Deleted TSKs: $0",
+                                      JoinStrings(entry_ids, ", "));
+  return Status::OK();
 }
 
 struct DeferredAssignmentActions {

diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
@@ -851,7 +851,8 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   Status LoadTspkEntries(std::vector<security::TokenSigningPublicKeyPB>* keys);
 
   // Delete TSK entries with the specified entry identifiers
-  // (identifiers correspond to the 'entry_id' column).
+  // (identifiers correspond to the 'entry_id' column). The 'entry_ids'
+  // container must not be empty.
   Status DeleteTskEntries(const std::set<std::string>& entry_ids);
 
   Status ApplyAlterSchemaSteps(const SysTablesEntryPB& current_pb,

diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc
@@ -743,19 +743,19 @@ Status SysCatalogTable::AddTskEntry(const SysTskEntryPB& entry) {
 }
 
 Status SysCatalogTable::RemoveTskEntries(const set<string>& entry_ids) {
+  CHECK(!entry_ids.empty());
   WriteRequestPB req;
-  WriteResponsePB resp;
-
   req.set_tablet_id(kSysCatalogTabletId);
+  RowOperationsPBEncoder enc(req.mutable_row_operations());
   CHECK_OK(SchemaToPB(schema_, req.mutable_schema()));
   for (const auto& id : entry_ids) {
     KuduPartialRow row(&schema_);
     CHECK_OK(row.SetInt8(kSysCatalogTableColType, TSK_ENTRY));
     CHECK_OK(row.SetStringNoCopy(kSysCatalogTableColId, id));
-    RowOperationsPBEncoder enc(req.mutable_row_operations());
     enc.Add(RowOperationsPB::DELETE, row);
   }
 
+  WriteResponsePB resp;
   return SyncWrite(&req, &resp);
 }
 

diff --git a/src/kudu/master/sys_catalog.h b/src/kudu/master/sys_catalog.h
@@ -186,7 +186,8 @@ class SysCatalogTable {
   Status AddTskEntry(const SysTskEntryPB& entry);
 
   // Remove TSK (Token Signing Key) entries with the specified entry identifiers
-  // (as in 'entry_id' column) from the system table.
+  // (as in 'entry_id' column) from the system table. The container of the
+  // entry identifiers must not be empty.
   Status RemoveTskEntries(const std::set<std::string>& entry_ids);
 
   // Return the underlying TabletReplica instance hosting the metadata.