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 2017/02/28 21:25:49 UTC
[2/5] kudu git commit: KUDU-1899. Fix support for empty string keys
KUDU-1899. Fix support for empty string keys
We've never prevented users from inserting a row with "" as a key. However, a
faulty assertion in diskrowset.cc caused the tablet server to crash on flush in
this case.
This adds a new end-to-end test via the client which inserts, updates, and deletes
a row with a "" key. It caused a crash prior to this patch.
Change-Id: I779f25afe6d39d91067b1e7c1238797ec2ac0295
Reviewed-on: http://gerrit.cloudera.org:8080/6163
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c11a315e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c11a315e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c11a315e
Branch: refs/heads/master
Commit: c11a315e73a20211caf71f2b3797e71af8449af4
Parents: 8f54f60
Author: Todd Lipcon <to...@apache.org>
Authored: Sun Feb 26 23:39:20 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Feb 28 20:52:51 2017 +0000
----------------------------------------------------------------------
src/kudu/client/client-test.cc | 80 +++++++++++++++++++++++++++++++++++++
src/kudu/tablet/diskrowset.cc | 3 +-
2 files changed, 81 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/c11a315e/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index bc57d6d..f7c8e0a 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -3950,6 +3950,86 @@ TEST_F(ClientTest, TestInsertTooLongEncodedPrimaryKey) {
errors[0]->status().ToString());
}
+// Test trying to insert a row with an empty string PK.
+// Regression test for KUDU-1899.
+TEST_F(ClientTest, TestInsertEmptyPK) {
+ const string kLongValue(10000, 'x');
+ const char* kTableName = "empty-pk";
+
+ // Create and open a table with a three-column composite PK.
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ KuduSchema schema;
+ KuduSchemaBuilder schema_builder;
+ schema_builder.AddColumn("k1")->Type(KuduColumnSchema::STRING)->NotNull();
+ schema_builder.AddColumn("v1")->Type(KuduColumnSchema::STRING)->NotNull();
+ schema_builder.SetPrimaryKey({"k1"});
+ ASSERT_OK(schema_builder.Build(&schema));
+ ASSERT_OK(table_creator->table_name(kTableName)
+ .schema(&schema)
+ .num_replicas(1)
+ .set_range_partition_columns({ "k1" })
+ .Create());
+ shared_ptr<KuduTable> table;
+ ASSERT_OK(client_->OpenTable(kTableName, &table));
+
+ // Find the tablet.
+ scoped_refptr<TabletPeer> tablet_peer;
+ ASSERT_TRUE(cluster_->mini_tablet_server(0)->server()->tablet_manager()->LookupTablet(
+ GetFirstTabletId(table.get()), &tablet_peer));
+
+ // Utility function to get the current value of the row.
+ const auto ReadRowAsString = [&]() {
+ vector<string> rows;
+ ScanTableToStrings(table.get(), &rows);
+ if (rows.empty()) return string("<none>");
+ CHECK_EQ(1, rows.size());
+ return rows[0];
+ };
+
+ shared_ptr<KuduSession> session(client_->NewSession());
+ ASSERT_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH));
+
+ // Insert a row with empty primary key.
+ {
+ unique_ptr<KuduInsert> insert(table->NewInsert());
+ ASSERT_OK(insert->mutable_row()->SetString(0, ""));
+ ASSERT_OK(insert->mutable_row()->SetString(1, "initial"));
+ ASSERT_OK(session->Apply(insert.release()));
+ ASSERT_OK(session->Flush());
+ }
+ ASSERT_EQ("(string k1=\"\", string v1=\"initial\")", ReadRowAsString());
+
+ // Make sure that Flush works properly, and the data is still readable.
+ ASSERT_OK(tablet_peer->tablet()->Flush());
+ ASSERT_EQ("(string k1=\"\", string v1=\"initial\")", ReadRowAsString());
+
+ // Perform an update.
+ {
+ unique_ptr<KuduUpdate> update(table->NewUpdate());
+ ASSERT_OK(update->mutable_row()->SetString(0, ""));
+ ASSERT_OK(update->mutable_row()->SetString(1, "updated"));
+ ASSERT_OK(session->Apply(update.release()));
+ ASSERT_OK(session->Flush());
+ }
+ ASSERT_EQ("(string k1=\"\", string v1=\"updated\")", ReadRowAsString());
+ ASSERT_OK(tablet_peer->tablet()->FlushAllDMSForTests());
+ ASSERT_EQ("(string k1=\"\", string v1=\"updated\")", ReadRowAsString());
+ ASSERT_OK(tablet_peer->tablet()->Compact(tablet::Tablet::FORCE_COMPACT_ALL));
+ ASSERT_EQ("(string k1=\"\", string v1=\"updated\")", ReadRowAsString());
+
+ // Perform a delete.
+ {
+ unique_ptr<KuduDelete> del(table->NewDelete());
+ ASSERT_OK(del->mutable_row()->SetString(0, ""));
+ ASSERT_OK(session->Apply(del.release()));
+ ASSERT_OK(session->Flush());
+ }
+ ASSERT_EQ("<none>", ReadRowAsString());
+ ASSERT_OK(tablet_peer->tablet()->FlushAllDMSForTests());
+ ASSERT_EQ("<none>", ReadRowAsString());
+ ASSERT_OK(tablet_peer->tablet()->Compact(tablet::Tablet::FORCE_COMPACT_ALL));
+ ASSERT_EQ("<none>", ReadRowAsString());
+}
// Check the behavior of the latest observed timestamp when performing
// write and read operations.
http://git-wip-us.apache.org/repos/asf/kudu/blob/c11a315e/src/kudu/tablet/diskrowset.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc
index 06b3897..014ecee 100644
--- a/src/kudu/tablet/diskrowset.cc
+++ b/src/kudu/tablet/diskrowset.cc
@@ -186,7 +186,7 @@ Status DiskRowSetWriter::AppendBlock(const RowBlock &block) {
}
#ifndef NDEBUG
- CHECK_LT(Slice(prev_key).compare(enc_key), 0)
+ CHECK(prev_key.size() == 0 || Slice(prev_key).compare(enc_key) < 0)
<< KUDU_REDACT(enc_key.ToDebugString()) << " appended to file not > previous key "
<< KUDU_REDACT(Slice(prev_key).ToDebugString());
#endif
@@ -214,7 +214,6 @@ Status DiskRowSetWriter::FinishAndReleaseBlocks(ScopedWritableBlockCloser* close
}
// Save the last encoded (max) key
- CHECK_GT(last_encoded_key_.size(), 0);
Slice last_enc_slice(last_encoded_key_);
std::string first_encoded_key =
key_index_writer()->GetMetaValueOrDie(DiskRowSet::kMinKeyMetaEntryName);