You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/10/02 19:09:05 UTC
[1/2] kudu git commit: KUDU-2159 Add metric for upserts converted to
updates
Repository: kudu
Updated Branches:
refs/heads/master bec2a2463 -> 5ca7deae7
KUDU-2159 Add metric for upserts converted to updates
This adds a metric tracking the number of successful upsert operations
that were applied as updates, per tablet, counting since server start.
Change-Id: Iefdee21fbfdfd3c9c8dca4c06d635b89bcf15d72
Reviewed-on: http://gerrit.cloudera.org:8080/8177
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3ec8834a
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3ec8834a
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3ec8834a
Branch: refs/heads/master
Commit: 3ec8834a43ba3ee5728fba0a3ed774c945279b7b
Parents: bec2a24
Author: Will Berkeley <wd...@apache.org>
Authored: Fri Sep 29 11:01:16 2017 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Mon Oct 2 16:30:48 2017 +0000
----------------------------------------------------------------------
src/kudu/tablet/tablet-test.cc | 11 +++++++++--
src/kudu/tablet/tablet.cc | 3 +++
src/kudu/tablet/tablet_metrics.cc | 5 +++++
src/kudu/tablet/tablet_metrics.h | 1 +
4 files changed, 18 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/3ec8834a/src/kudu/tablet/tablet-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet-test.cc b/src/kudu/tablet/tablet-test.cc
index e5886be..3b9c954 100644
--- a/src/kudu/tablet/tablet-test.cc
+++ b/src/kudu/tablet/tablet-test.cc
@@ -53,6 +53,7 @@
#include "kudu/tablet/tablet.h"
#include "kudu/tablet/tablet.pb.h"
#include "kudu/tablet/tablet_metadata.h"
+#include "kudu/tablet/tablet_metrics.h" // IWYU pragma: keep
#include "kudu/util/faststring.h"
#include "kudu/util/jsonwriter.h"
#include "kudu/util/metrics.h"
@@ -719,10 +720,15 @@ TYPED_TEST(TestTablet, TestInsertsPersist) {
TYPED_TEST(TestTablet, TestUpsert) {
vector<string> rows;
+ const auto& upserts_as_updates = this->tablet()->metrics()->upserts_as_updates;
+
+ // Upsert a new row.
this->UpsertTestRows(0, 1, 1000);
+ EXPECT_EQ(0, upserts_as_updates->value());
- // UPSERT a row that is in MRS.
+ // Upsert a row that is in the MRS.
this->UpsertTestRows(0, 1, 1001);
+ EXPECT_EQ(1, upserts_as_updates->value());
ASSERT_OK(this->IterateToStringList(&rows));
EXPECT_EQ(vector<string>{ this->setup_.FormatDebugRow(0, 1001, false) }, rows);
@@ -732,8 +738,9 @@ TYPED_TEST(TestTablet, TestUpsert) {
ASSERT_OK(this->IterateToStringList(&rows));
EXPECT_EQ(vector<string>{ this->setup_.FormatDebugRow(0, 1001, false) }, rows);
- // UPSERT a row that is in DRS.
+ // Upsert a row that is in the DRS.
this->UpsertTestRows(0, 1, 1002);
+ EXPECT_EQ(2, upserts_as_updates->value());
ASSERT_OK(this->IterateToStringList(&rows));
EXPECT_EQ(vector<string>{ this->setup_.FormatDebugRow(0, 1002, false) }, rows);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/3ec8834a/src/kudu/tablet/tablet.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet.cc b/src/kudu/tablet/tablet.cc
index 76fb887..0b16f07 100644
--- a/src/kudu/tablet/tablet.cc
+++ b/src/kudu/tablet/tablet.cc
@@ -611,6 +611,9 @@ Status Tablet::ApplyUpsertAsUpdate(WriteTransactionState* tx_state,
result.get());
CHECK(!s.IsNotFound());
if (s.ok()) {
+ if (metrics_) {
+ metrics_->upserts_as_updates->Increment();
+ }
upsert->SetMutateSucceeded(std::move(result));
} else {
upsert->SetFailed(s);
http://git-wip-us.apache.org/repos/asf/kudu/blob/3ec8834a/src/kudu/tablet/tablet_metrics.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_metrics.cc b/src/kudu/tablet/tablet_metrics.cc
index d73ccd4..b5469e9 100644
--- a/src/kudu/tablet/tablet_metrics.cc
+++ b/src/kudu/tablet/tablet_metrics.cc
@@ -94,6 +94,10 @@ METRIC_DEFINE_counter(tablet, scanner_bytes_scanned_from_disk, "Scanner Bytes Sc
METRIC_DEFINE_counter(tablet, insertions_failed_dup_key, "Duplicate Key Inserts",
kudu::MetricUnit::kRows,
"Number of inserts which failed because the key already existed");
+METRIC_DEFINE_counter(tablet, upserts_as_updates, "Upserts converted into updates",
+ kudu::MetricUnit::kRows,
+ "Number of upserts which were applied as updates because the key already "
+ "existed.");
METRIC_DEFINE_counter(tablet, scans_started, "Scans Started",
kudu::MetricUnit::kScanners,
"Number of scanners which have been started on this tablet");
@@ -263,6 +267,7 @@ TabletMetrics::TabletMetrics(const scoped_refptr<MetricEntity>& entity)
MINIT(rows_updated),
MINIT(rows_deleted),
MINIT(insertions_failed_dup_key),
+ MINIT(upserts_as_updates),
MINIT(scanner_rows_returned),
MINIT(scanner_cells_returned),
MINIT(scanner_bytes_returned),
http://git-wip-us.apache.org/repos/asf/kudu/blob/3ec8834a/src/kudu/tablet/tablet_metrics.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_metrics.h b/src/kudu/tablet/tablet_metrics.h
index d7f3b38..6fa5a64 100644
--- a/src/kudu/tablet/tablet_metrics.h
+++ b/src/kudu/tablet/tablet_metrics.h
@@ -48,6 +48,7 @@ struct TabletMetrics {
scoped_refptr<Counter> rows_updated;
scoped_refptr<Counter> rows_deleted;
scoped_refptr<Counter> insertions_failed_dup_key;
+ scoped_refptr<Counter> upserts_as_updates;
scoped_refptr<Counter> scanner_rows_returned;
scoped_refptr<Counter> scanner_cells_returned;
scoped_refptr<Counter> scanner_bytes_returned;
[2/2] kudu git commit: KUDU-2132 Error dropping columns renamed to
old key column names
Posted by da...@apache.org.
KUDU-2132 Error dropping columns renamed to old key column names
Suppose a table has two columns 'a' and 'b', and 'a' is the primary
key. Consider the following sequence of alter table steps, done as
part of one alter table request:
1. Rename the key column 'a' to 'c'.
2. Rename 'b' to 'a', the previous name of the key column.
3. Drop 'a'.
This is a valid sequence, but previously we were rejecting it,
saying that a primary key column was being dropped, even though
that isn't the case.
The problem is that CatalogManager::ApplyAlterSchemaSteps
checked whether a column was a key column by checking by
name against the table's schema before any alter steps were
applied. The fix is to check against the schema with the
previous alter steps applied.
Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2
Reviewed-on: http://gerrit.cloudera.org:8080/8188
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/5ca7deae
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/5ca7deae
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/5ca7deae
Branch: refs/heads/master
Commit: 5ca7deae7483d9e6427d795b806680ec3c08a73c
Parents: 3ec8834
Author: Will Berkeley <wd...@apache.org>
Authored: Fri Sep 29 15:17:17 2017 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Mon Oct 2 18:58:16 2017 +0000
----------------------------------------------------------------------
src/kudu/common/schema.h | 10 ++++++++++
src/kudu/integration-tests/alter_table-test.cc | 9 +++++++++
src/kudu/master/catalog_manager.cc | 2 +-
src/kudu/master/catalog_manager.h | 1 +
4 files changed, 21 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/common/schema.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/schema.h b/src/kudu/common/schema.h
index d9fe0ab..724b9c0 100644
--- a/src/kudu/common/schema.h
+++ b/src/kudu/common/schema.h
@@ -891,6 +891,16 @@ class SchemaBuilder {
return next_id_;
}
+ // Return true if the column named 'col_name' is a key column.
+ // Return false if the column is not a key column, or if
+ // the column is not in the builder.
+ bool is_key_column(const StringPiece col_name) const {
+ for (int i = 0; i < num_key_columns_; i++) {
+ if (cols_[i].name() == col_name) return true;
+ }
+ return false;
+ }
+
Schema Build() const { return Schema(cols_, col_ids_, num_key_columns_); }
Schema BuildWithoutIds() const { return Schema(cols_, num_key_columns_); }
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/integration-tests/alter_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc
index bc2a501..55caef0 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -2102,4 +2102,13 @@ TEST_F(AlterTableTest, TestRenameStillCreatingTable) {
}
}
+// Regression test for KUDU-2132.
+TEST_F(AlterTableTest, TestKUDU2132) {
+ unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
+ table_alterer->AlterColumn("c0")->RenameTo("primaryKeyRenamed");
+ table_alterer->AlterColumn("c1")->RenameTo("c0");
+ table_alterer->DropColumn("c0");
+ ASSERT_OK(table_alterer->Alter());
+}
+
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 7c95d43..e073a97 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1656,7 +1656,7 @@ Status CatalogManager::ApplyAlterSchemaSteps(const SysTablesEntryPB& current_pb,
return Status::InvalidArgument("DROP_COLUMN missing column info");
}
- if (cur_schema.is_key_column(step.drop_column().name())) {
+ if (builder.is_key_column(step.drop_column().name())) {
return Status::InvalidArgument("cannot remove a key column",
step.drop_column().name());
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index f2b1798..0a06912 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -22,6 +22,7 @@
#include <iosfwd>
#include <map>
#include <memory>
+#include <mutex>
#include <set>
#include <string>
#include <unordered_map>