kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [2/2] incubator-kudu git commit: Add TableCreator::add_range_split and deprecate TableCreator::split_rows
Date Thu, 02 Jun 2016 17:23:47 GMT
Add TableCreator::add_range_split and deprecate TableCreator::split_rows

TableCreator::split_rows takes a vector<const KuduPartialRow*>, which makes it
very difficult for the calling application to do proper error handling with
cleanup when setting the fields of the partial row. Taking the range split rows
one by one allows the application to wrap the partial row in a unique_ptr 'end
to end', and handle errors correctly. client-test has been updated to use the
new API, and now handles errors correctly when initializing split rows.

Change-Id: I72cc152ff4b01834abd3c7b6fc32980ceb789c69
Reviewed-on: http://gerrit.cloudera.org:8080/3275
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <adar@cloudera.com>


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

Branch: refs/heads/master
Commit: fc1619be9cd3151ce78ba57412049884b01c4ba9
Parents: c820c77
Author: Dan Burkert <dan@cloudera.com>
Authored: Wed Jun 1 15:07:05 2016 -0700
Committer: Dan Burkert <dan@cloudera.com>
Committed: Thu Jun 2 17:22:20 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/client-test.cc            | 41 +++++++++++++-------------
 src/kudu/client/client.cc                 | 14 +++++++--
 src/kudu/client/client.h                  | 15 ++++++----
 src/kudu/client/table_creator-internal.cc |  1 -
 src/kudu/client/table_creator-internal.h  |  2 +-
 5 files changed, 43 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/fc1619be/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index b0725ed..1aae285 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -19,8 +19,9 @@
 #include <gflags/gflags.h>
 #include <glog/stl_logging.h>
 
-#include <vector>
 #include <algorithm>
+#include <memory>
+#include <vector>
 
 #include "kudu/client/callbacks.h"
 #include "kudu/client/client.h"
@@ -77,8 +78,9 @@ METRIC_DECLARE_histogram(handler_latency_kudu_master_MasterService_GetMasterRegi
 METRIC_DECLARE_histogram(handler_latency_kudu_master_MasterService_GetTableLocations);
 METRIC_DECLARE_histogram(handler_latency_kudu_master_MasterService_GetTabletLocations);
 
-using std::string;
 using std::set;
+using std::string;
+using std::unique_ptr;
 using std::vector;
 
 namespace kudu {
@@ -110,7 +112,7 @@ class ClientTest : public KuduTest {
     FLAGS_enable_data_block_fsync = false; // Keep unit tests fast.
   }
 
-  virtual void SetUp() OVERRIDE {
+  void SetUp() override {
     KuduTest::SetUp();
 
     // Reduce the TS<->Master heartbeat interval
@@ -127,20 +129,19 @@ class ClientTest : public KuduTest {
                      .Build(&client_));
 
     ASSERT_NO_FATAL_FAILURE(CreateTable(kTableName, 1, GenerateSplitRows(), &client_table_));
-    ASSERT_NO_FATAL_FAILURE(CreateTable(kTable2Name, 1, vector<const KuduPartialRow*>(),
-                                        &client_table2_));
+    ASSERT_NO_FATAL_FAILURE(CreateTable(kTable2Name, 1, {}, &client_table2_));
   }
 
   // Generate a set of split rows for tablets used in this test.
-  vector<const KuduPartialRow*> GenerateSplitRows() {
-    vector<const KuduPartialRow*> rows;
-    KuduPartialRow* row = schema_.NewRow();
+  vector<unique_ptr<KuduPartialRow>> GenerateSplitRows() {
+    vector<unique_ptr<KuduPartialRow>> rows;
+    unique_ptr<KuduPartialRow> row(schema_.NewRow());
     CHECK_OK(row->SetInt32(0, 9));
-    rows.push_back(row);
+    rows.push_back(std::move(row));
     return rows;
   }
 
-  virtual void TearDown() OVERRIDE {
+  void TearDown() override {
     if (cluster_) {
       cluster_->Shutdown();
       cluster_.reset();
@@ -381,7 +382,7 @@ class ClientTest : public KuduTest {
   // (or single tablet if 'split_rows' is empty).
   void CreateTable(const string& table_name,
                    int num_replicas,
-                   const vector<const KuduPartialRow*>& split_rows,
+                   vector<unique_ptr<KuduPartialRow>> split_rows,
                    shared_ptr<KuduTable>* table) {
 
     bool added_replicas = false;
@@ -396,11 +397,14 @@ class ClientTest : public KuduTest {
     }
 
     gscoped_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+    for (auto& split_row : split_rows) {
+      table_creator->add_range_split(split_row.release());
+    }
+
     ASSERT_OK(table_creator->table_name(table_name)
                             .schema(&schema_)
                             .num_replicas(num_replicas)
                             .set_range_partition_columns({ "key" })
-                            .split_rows(split_rows)
                             .Create());
 
     ASSERT_OK(client_->OpenTable(table_name, table));
@@ -609,15 +613,15 @@ TEST_F(ClientTest, TestScanAtFutureTimestamp) {
 TEST_F(ClientTest, TestScanMultiTablet) {
   // 5 tablets, each with 10 rows worth of space.
   gscoped_ptr<KuduPartialRow> row(schema_.NewRow());
-  vector<const KuduPartialRow*> rows;
+  vector<unique_ptr<KuduPartialRow>> rows;
   for (int i = 1; i < 5; i++) {
-    KuduPartialRow* row = schema_.NewRow();
+    unique_ptr<KuduPartialRow> row(schema_.NewRow());
     CHECK_OK(row->SetInt32(0, i * 10));
-    rows.push_back(row);
+    rows.push_back(std::move(row));
   }
   gscoped_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
   shared_ptr<KuduTable> table;
-  ASSERT_NO_FATAL_FAILURE(CreateTable("TestScanMultiTablet", 1, rows, &table));
+  ASSERT_NO_FATAL_FAILURE(CreateTable("TestScanMultiTablet", 1, std::move(rows), &table));
 
   // Insert rows with keys 12, 13, 15, 17, 22, 23, 25, 27...47 into each
   // tablet, except the first which is empty.
@@ -2292,10 +2296,7 @@ TEST_F(ClientTest, TestReplicatedTabletWritesWithLeaderElection) {
   const int kNumReplicas = 3;
 
   shared_ptr<KuduTable> table;
-  ASSERT_NO_FATAL_FAILURE(CreateTable(kReplicatedTable,
-                                      kNumReplicas,
-                                      vector<const KuduPartialRow*>(),
-                                      &table));
+  ASSERT_NO_FATAL_FAILURE(CreateTable(kReplicatedTable, kNumReplicas, {}, &table));
 
   // Insert some data.
   ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), kNumRowsToWrite));

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/fc1619be/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 5b6d900..781e917 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -467,8 +467,15 @@ KuduTableCreator& KuduTableCreator::set_range_partition_columns(
   return *this;
 }
 
+KuduTableCreator& KuduTableCreator::add_range_split(KuduPartialRow* split_row) {
+  data_->range_splits_.emplace_back(split_row);
+  return *this;
+}
+
 KuduTableCreator& KuduTableCreator::split_rows(const vector<const KuduPartialRow*>&
rows) {
-  data_->split_rows_ = rows;
+  for (const KuduPartialRow* row : rows) {
+    data_->range_splits_.emplace_back(const_cast<KuduPartialRow*>(row));
+  }
   return *this;
 }
 
@@ -512,7 +519,10 @@ Status KuduTableCreator::Create() {
 
   RowOperationsPBEncoder encoder(req.mutable_split_rows_range_bounds());
 
-  for (const KuduPartialRow* row : data_->split_rows_) {
+  for (const auto& row : data_->range_splits_) {
+    if (!row) {
+      return Status::InvalidArgument("range split row must not be null");
+    }
     encoder.Add(RowOperationsPB::SPLIT_ROW, *row);
   }
   req.mutable_partition_schema()->CopyFrom(data_->partition_schema_);

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/fc1619be/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 99664aa..f7f3702 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -352,16 +352,19 @@ class KUDU_EXPORT KuduTableCreator {
   // partitions.
   KuduTableCreator& set_range_partition_columns(const std::vector<std::string>&
columns);
 
-  // Sets the rows on which to pre-split the table.
-  // The table creator takes ownership of the rows.
+  // Adds a range partition split at the provided row.
   //
-  // If any provided row is missing a value for any of the range partition
-  // columns, the logical minimum value for that column type will be used by
-  // default.
+  // The table creator takes ownership of the row.
   //
-  // If not provided, no range-based pre-splitting is performed.
+  // If the row is missing a value for any of the range partition columns, the
+  // logical minimum value for that column type will be used by default.
+  //
+  // If no range split rows are added, no range pre-splitting is performed.
   //
   // Optional.
+  KuduTableCreator& add_range_split(KuduPartialRow* split_row);
+
+  // DEPRECATED: use add_range_split
   KuduTableCreator& split_rows(const std::vector<const KuduPartialRow*>& split_rows);
 
   // Sets the number of replicas for each tablet in the table.

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/fc1619be/src/kudu/client/table_creator-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/table_creator-internal.cc b/src/kudu/client/table_creator-internal.cc
index f1b8b42..36311a9 100644
--- a/src/kudu/client/table_creator-internal.cc
+++ b/src/kudu/client/table_creator-internal.cc
@@ -32,7 +32,6 @@ KuduTableCreator::Data::Data(KuduClient* client)
 }
 
 KuduTableCreator::Data::~Data() {
-  STLDeleteElements(&split_rows_);
 }
 
 } // namespace client

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/fc1619be/src/kudu/client/table_creator-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/table_creator-internal.h b/src/kudu/client/table_creator-internal.h
index 7aff47a..3f8d7e0 100644
--- a/src/kudu/client/table_creator-internal.h
+++ b/src/kudu/client/table_creator-internal.h
@@ -38,7 +38,7 @@ class KuduTableCreator::Data {
 
   const KuduSchema* schema_;
 
-  std::vector<const KuduPartialRow*> split_rows_;
+  std::vector<std::unique_ptr<KuduPartialRow>> range_splits_;
 
   PartitionSchemaPB partition_schema_;
 


Mime
View raw message