kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/4] incubator-kudu git commit: TableInfo::RemoveTablet should use partition_key_start
Date Sat, 16 Jan 2016 01:33:15 GMT
TableInfo::RemoveTablet should use partition_key_start

TableInfo::RemoveTablet try to remove tablet by tablet_id, but
tablet_map_ uses partition_key_start as key, so it does not work.
This commit fix this bug and add unit test for RemoveTablet.

Change-Id: I3b34ea8d4e04dd8a8b567dfc451edeabe34c7e2b
Reviewed-on: http://gerrit.cloudera.org:8080/1768
Reviewed-by: Todd Lipcon <todd@apache.org>
Tested-by: Internal Jenkins


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

Branch: refs/heads/master
Commit: a80b83fe5e6a3cac1c310c2764fca0b6ca4a60e3
Parents: c8049c1
Author: Binglin Chang <decstery@gmail.com>
Authored: Tue Jan 12 15:51:54 2016 +0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Sat Jan 16 00:42:45 2016 +0000

----------------------------------------------------------------------
 src/kudu/master/catalog_manager-test.cc |  8 ++++++++
 src/kudu/master/catalog_manager.cc      | 14 +++++---------
 src/kudu/master/catalog_manager.h       |  6 +++---
 3 files changed, 16 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/a80b83fe/src/kudu/master/catalog_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager-test.cc b/src/kudu/master/catalog_manager-test.cc
index d7d3ea6..2dee461 100644
--- a/src/kudu/master/catalog_manager-test.cc
+++ b/src/kudu/master/catalog_manager-test.cc
@@ -32,6 +32,7 @@ using strings::Substitute;
 TEST(TableInfoTest, TestAssignmentRanges) {
   const string table_id = CURRENT_TEST_NAME();
   scoped_refptr<TableInfo> table(new TableInfo(table_id));
+  vector<scoped_refptr<TabletInfo> > tablets;
 
   // Define & create the splits.
   const int kNumSplits = 3;
@@ -50,6 +51,8 @@ TEST(TableInfoTest, TestAssignmentRanges) {
     meta_lock.mutable_data()->pb.set_state(SysTabletsEntryPB::RUNNING);
 
     table->AddTablet(tablet);
+    meta_lock.Commit();
+    tablets.push_back(make_scoped_refptr(tablet));
   }
 
   // Ensure they give us what we are expecting.
@@ -73,6 +76,11 @@ TEST(TableInfoTest, TestAssignmentRanges) {
     ASSERT_EQ(tablet_id, (*tablets_in_range.begin())->tablet_id());
     LOG(INFO) << "Key " << start_key << " found in tablet " << tablet_id;
   }
+
+  BOOST_FOREACH(const scoped_refptr<TabletInfo>& tablet, tablets) {
+    ASSERT_TRUE(table->RemoveTablet(
+        tablet->metadata().state().pb.partition().partition_key_start()));
+  }
 }
 
 } // namespace master

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/a80b83fe/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 8a3b77b..8b1cfb8 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -2735,8 +2735,9 @@ Status CatalogManager::ProcessPendingAssignments(
     vector<string> tablet_ids_to_remove;
     for (scoped_refptr<TabletInfo>& new_tablet : new_tablets) {
       TableInfo* table = new_tablet->table().get();
-      TableMetadataLock l_table(table, TableMetadataLock::WRITE);
-      if (table->RemoveTablet(new_tablet->tablet_id())) {
+      TableMetadataLock l_table(table, TableMetadataLock::READ);
+      if (table->RemoveTablet(
+          new_tablet->metadata().dirty().pb.partition().partition_key_start())) {
         VLOG(1) << "Removed tablet " << new_tablet->tablet_id() << "
from "
             "table " << l_table.data().name();
       }
@@ -3113,14 +3114,9 @@ std::string TableInfo::ToString() const {
   return Substitute("$0 [id=$1]", l.data().pb.name(), table_id_);
 }
 
-bool TableInfo::RemoveTablet(const string& tablet_id) {
+bool TableInfo::RemoveTablet(const std::string& partition_key_start) {
   boost::lock_guard<simple_spinlock> l(lock_);
-  if (tablet_map_.find(tablet_id) != tablet_map_.end()) {
-    CHECK_EQ(tablet_map_.erase(tablet_id), 1)
-        << "Unable to erase tablet " << tablet_id << " from TableInfo.";
-    return true;
-  }
-  return false;
+  return EraseKeyReturnValuePtr(&tablet_map_, partition_key_start) != NULL;
 }
 
 void TableInfo::AddTablet(TabletInfo *tablet) {

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/a80b83fe/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index 11c4b9f..f6c7db0 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -214,9 +214,9 @@ class TableInfo : public RefCountedThreadSafe<TableInfo> {
   // Add multiple tablets to this table.
   void AddTablets(const std::vector<TabletInfo*>& tablets);
 
-  // Return true if tablet with 'tablet_id' has been removed from
-  // 'tablet_map_' below.
-  bool RemoveTablet(const std::string& tablet_id);
+  // Return true if tablet with 'partition_key_start' has been
+  // removed from 'tablet_map_' below.
+  bool RemoveTablet(const std::string& partition_key_start);
 
   // This only returns tablets which are in RUNNING state.
   void GetTabletsInRange(const GetTableLocationsRequestPB* req,


Mime
View raw message