You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/10/20 00:40:53 UTC

[1/2] kudu git commit: KUDU-2170 Masters can start with duplicates specified

Repository: kudu
Updated Branches:
  refs/heads/master f2eb6c277 -> 4172983fd


KUDU-2170 Masters can start with duplicates specified

Users can specify duplicate masters in --master_addresses, where a
duplicate includes duplicated names and the same server known by
different names. This doesn't cause problems as far as I know, but it
looks scary because, for example, the web ui shows extra masters listed in
/masters. This is because /masters shows one master per result from
ListMasters. This patch deduplicates the output of ListMasters by UUID.

It also warns on start if there are exact string duplicates in
--master_addresses, at the time the master compares on-disk and
configured master addresses, but before the masters are ready to
resolve UUIDs.

Also, the error message when on-disk and configured masters don't match
is confusing. This patch makes it easy to understand.

Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Reviewed-on: http://gerrit.cloudera.org:8080/8341
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>


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

Branch: refs/heads/master
Commit: c7fa4aaf704e4035843bc8fe20e5ecada2ed1acc
Parents: f2eb6c2
Author: Will Berkeley <wd...@apache.org>
Authored: Thu Oct 19 13:34:02 2017 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Fri Oct 20 00:37:55 2017 +0000

----------------------------------------------------------------------
 src/kudu/master/master.cc      | 14 +++++++++++++-
 src/kudu/master/sys_catalog.cc | 13 +++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c7fa4aaf/src/kudu/master/master.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 4a88007..0b2d7d9 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -18,8 +18,11 @@
 #include "kudu/master/master.h"
 
 #include <algorithm>
+#include <iterator>
 #include <memory>
 #include <ostream>
+#include <set>
+#include <string>
 #include <type_traits>
 #include <vector>
 
@@ -28,10 +31,12 @@
 
 #include "kudu/cfile/block_cache.h"
 #include "kudu/common/wire_protocol.h"
+#include "kudu/common/wire_protocol.pb.h"
 #include "kudu/consensus/metadata.pb.h"
 #include "kudu/fs/fs_manager.h"
 #include "kudu/gutil/bind.h"
 #include "kudu/gutil/bind_helpers.h"
+#include "kudu/gutil/map-util.h"
 #include "kudu/gutil/move.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/catalog_manager.h"
@@ -303,6 +308,12 @@ Status Master::ListMasters(std::vector<ServerEntryPB>* masters) const {
     return Status::OK();
   }
 
+  // Since --master_addresses may contain duplicates, including different names
+  // for the same server, we deduplicate the masters by UUID here.
+  auto uuid_cmp = [](const ServerEntryPB& left, const ServerEntryPB& right) {
+    return left.instance_id().permanent_uuid() < right.instance_id().permanent_uuid();
+  };
+  std::set<ServerEntryPB, decltype(uuid_cmp)> masters_by_uuid(uuid_cmp);
   for (const HostPort& peer_addr : opts_.master_addresses) {
     ServerEntryPB peer_entry;
     Status s = GetMasterEntryForHost(messenger_, peer_addr, &peer_entry);
@@ -313,9 +324,10 @@ Status Master::ListMasters(std::vector<ServerEntryPB>* masters) const {
       LOG(WARNING) << s.ToString();
       StatusToPB(s, peer_entry.mutable_error());
     }
-    masters->push_back(peer_entry);
+    InsertIfNotPresent(&masters_by_uuid, peer_entry);
   }
 
+  std::copy(masters_by_uuid.begin(), masters_by_uuid.end(), std::back_inserter(*masters));
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c7fa4aaf/src/kudu/master/sys_catalog.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc
index fe16c96..7836380 100644
--- a/src/kudu/master/sys_catalog.cc
+++ b/src/kudu/master/sys_catalog.cc
@@ -24,6 +24,7 @@
 #include <memory>
 #include <ostream>
 #include <set>
+#include <string>
 #include <type_traits>
 #include <utility>
 #include <vector>
@@ -196,6 +197,11 @@ Status SysCatalogTable::Load(FsManager *fs_manager) {
     for (const auto& hp : master_->opts().master_addresses) {
       peer_addrs_from_opts.insert(hp.ToString());
     }
+    if (peer_addrs_from_opts.size() < master_->opts().master_addresses.size()) {
+      LOG(WARNING) << Substitute("Found duplicates in --master_addresses: "
+                                 "the unique set of addresses is $0",
+                                 JoinStrings(peer_addrs_from_opts, ", "));
+    }
     set<string> peer_addrs_from_disk;
     for (const auto& p : cstate.committed_config().peers()) {
       HostPort hp;
@@ -210,8 +216,11 @@ Status SysCatalogTable::Load(FsManager *fs_manager) {
                                   std::back_inserter(symm_diff));
     if (!symm_diff.empty()) {
       string msg = Substitute(
-          "on-disk and provided master lists are different: $0",
-          JoinStrings(symm_diff, " "));
+          "on-disk master list ($0) and provided master list ($1) differ. "
+          "Their symmetric difference is: $2",
+          JoinStrings(peer_addrs_from_opts, ", "),
+          JoinStrings(peer_addrs_from_disk, ", "),
+          JoinStrings(symm_diff, ", "));
       return Status::InvalidArgument(msg);
     }
   }


[2/2] kudu git commit: Integrate BlockCreationTransaction into MajorDeltaCompaction

Posted by ad...@apache.org.
Integrate BlockCreationTransaction into MajorDeltaCompaction

Previous commit 59ca360bf integrated BlockCreationTransaction with
RowSet compaction, so that only a single transaction is used to finalize
underlying blocks. However in major delta compaction, MultiColumnWriter
and DeltaFileWriter use separate creation transactions, which does not
take full advantage of BlockCreationTransaction.

This patch includes a minor improvement to incorporate
BlockCreationTransaction with MajorDeltaCompaction to use a single
transaction for a RowSet's major delta compaction.

Change-Id: I887e20643ac8e4e728f8f91450d2d5059cb33d20
Reviewed-on: http://gerrit.cloudera.org:8080/8325
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>


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

Branch: refs/heads/master
Commit: 4172983fd29f5e2fd9b322ff10c2b016f89a5acb
Parents: c7fa4aa
Author: hahao <ha...@cloudera.com>
Authored: Wed Oct 18 16:30:49 2017 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Fri Oct 20 00:40:38 2017 +0000

----------------------------------------------------------------------
 src/kudu/tablet/delta_compaction.cc    | 11 ++++++++---
 src/kudu/tablet/multi_column_writer.cc |  8 --------
 src/kudu/tablet/multi_column_writer.h  |  5 -----
 3 files changed, 8 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4172983f/src/kudu/tablet/delta_compaction.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/delta_compaction.cc b/src/kudu/tablet/delta_compaction.cc
index 0b7fecd..4c4a09c 100644
--- a/src/kudu/tablet/delta_compaction.cc
+++ b/src/kudu/tablet/delta_compaction.cc
@@ -53,6 +53,8 @@ using std::shared_ptr;
 
 namespace kudu {
 
+using fs::BlockCreationTransaction;
+using fs::BlockManager;
 using fs::CreateBlockOptions;
 using fs::WritableBlock;
 using std::string;
@@ -228,17 +230,20 @@ Status MajorDeltaCompaction::FlushRowSetAndDeltas() {
     nrows += n;
   }
 
-  RETURN_NOT_OK(base_data_writer_->Finish());
+  BlockManager* bm = fs_manager_->block_manager();
+  unique_ptr<BlockCreationTransaction> transaction = bm->NewCreationTransaction();
+  RETURN_NOT_OK(base_data_writer_->FinishAndReleaseBlocks(transaction.get()));
 
   if (redo_delta_mutations_written_ > 0) {
     new_redo_delta_writer_->WriteDeltaStats(redo_stats);
-    RETURN_NOT_OK(new_redo_delta_writer_->Finish());
+    RETURN_NOT_OK(new_redo_delta_writer_->FinishAndReleaseBlock(transaction.get()));
   }
 
   if (undo_delta_mutations_written_ > 0) {
     new_undo_delta_writer_->WriteDeltaStats(undo_stats);
-    RETURN_NOT_OK(new_undo_delta_writer_->Finish());
+    RETURN_NOT_OK(new_undo_delta_writer_->FinishAndReleaseBlock(transaction.get()));
   }
+  transaction->CommitCreatedBlocks();
 
   DVLOG(1) << "Applied all outstanding deltas for columns "
            << partial_schema_.ToString()

http://git-wip-us.apache.org/repos/asf/kudu/blob/4172983f/src/kudu/tablet/multi_column_writer.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/multi_column_writer.cc b/src/kudu/tablet/multi_column_writer.cc
index 1fc2671..15f70b5 100644
--- a/src/kudu/tablet/multi_column_writer.cc
+++ b/src/kudu/tablet/multi_column_writer.cc
@@ -38,7 +38,6 @@ namespace tablet {
 
 using cfile::CFileWriter;
 using fs::BlockCreationTransaction;
-using fs::BlockManager;
 using fs::CreateBlockOptions;
 using fs::WritableBlock;
 using std::unique_ptr;
@@ -119,13 +118,6 @@ Status MultiColumnWriter::AppendBlock(const RowBlock& block) {
   return Status::OK();
 }
 
-Status MultiColumnWriter::Finish() {
-  BlockManager* bm = fs_->block_manager();
-  unique_ptr<BlockCreationTransaction> transaction = bm->NewCreationTransaction();
-  RETURN_NOT_OK(FinishAndReleaseBlocks(transaction.get()));
-  return transaction->CommitCreatedBlocks();
-}
-
 Status MultiColumnWriter::FinishAndReleaseBlocks(
     BlockCreationTransaction* transaction) {
   CHECK(!finished_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/4172983f/src/kudu/tablet/multi_column_writer.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/multi_column_writer.h b/src/kudu/tablet/multi_column_writer.h
index 7d880f4..259d0bc 100644
--- a/src/kudu/tablet/multi_column_writer.h
+++ b/src/kudu/tablet/multi_column_writer.h
@@ -63,11 +63,6 @@ class MultiColumnWriter {
   // Note that the selection vector here is ignored.
   Status AppendBlock(const RowBlock& block);
 
-  // Close the in-progress files.
-  //
-  // The file's blocks may be retrieved using FlushedBlocks().
-  Status Finish();
-
   // Close the in-progress CFiles, finalizing the underlying writable
   // blocks and releasing them to 'transaction'.
   Status FinishAndReleaseBlocks(fs::BlockCreationTransaction* transaction);