You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/01/16 02:33:15 UTC
[2/4] incubator-kudu git commit: TableInfo::RemoveTablet should use
partition_key_start
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 <to...@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 <de...@gmail.com>
Authored: Tue Jan 12 15:51:54 2016 +0800
Committer: Todd Lipcon <to...@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,