You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2017/06/19 17:38:15 UTC

kudu git commit: alter_table-test: add a workaround for KUDU-791

Repository: kudu
Updated Branches:
  refs/heads/master d39781a5e -> 870343f0b


alter_table-test: add a workaround for KUDU-791

KUDU-791 describes the fact that our AlterTable operations do not fully
respect our consistency guarantees. This test, however, was making
assertions on the alter-table success immediately after issuing an alter
table command.

The fix just switches to ASSERT_EVENTUALLY. I looped the test 500 times
with the change[1] and all passed.

I also looped 1000 times with 4 CPU stress threads[2]. I got two
failures which seem to be related to some clock-related test issue
separate from this change.

[1] http://dist-test.cloudera.org/job?job_id=todd.1497399757.25356
[2] httpL//dist-test.cloudera.org/job?job_id=todd.1497577640.27529

Change-Id: I63769f7a95928a2f25ad1d35b6cb346fa3d13bd4
Reviewed-on: http://gerrit.cloudera.org:8080/7210
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>


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

Branch: refs/heads/master
Commit: 870343f0b1e9dc1117d5243682bf34b6e1b54acc
Parents: d39781a
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Jun 15 18:40:29 2017 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Mon Jun 19 17:37:03 2017 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/alter_table-test.cc | 32 ++++++++++++---------
 1 file changed, 19 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/870343f0/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 3360271..334626b 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -674,19 +674,21 @@ void AlterTableTest::ScanToStrings(vector<string>* rows) {
 // key (InsertRows swaps endianness so that we random-write instead of sequential-write)
 void AlterTableTest::VerifyRows(int start_row, int num_rows, VerifyPattern pattern) {
   shared_ptr<KuduTable> table;
-  CHECK_OK(client_->OpenTable(kTableName, &table));
+  ASSERT_OK(client_->OpenTable(kTableName, &table));
   KuduScanner scanner(table.get());
-  CHECK_OK(scanner.SetSelection(KuduClient::LEADER_ONLY));
-  CHECK_OK(scanner.Open());
+  // TODO(KUDU-791): we should probably be able to use a snapshot read here,
+  // but alter-table isn't all the way tied into the consistency-related code.
+  ASSERT_OK(scanner.SetSelection(KuduClient::LEADER_ONLY));
+  ASSERT_OK(scanner.Open());
 
   int verified = 0;
   vector<KuduRowResult> results;
   while (scanner.HasMoreRows()) {
-    CHECK_OK(scanner.NextBatch(&results));
+    ASSERT_OK(scanner.NextBatch(&results));
 
     for (const KuduRowResult& row : results) {
       int32_t key = 0;
-      CHECK_OK(row.GetInt32(0, &key));
+      ASSERT_OK(row.GetInt32(0, &key));
       int32_t row_idx = bswap_32(key);
       if (row_idx < start_row || row_idx >= start_row + num_rows) {
         // Outside the range we're verifying
@@ -699,21 +701,21 @@ void AlterTableTest::VerifyRows(int start_row, int num_rows, VerifyPattern patte
       }
 
       int32_t c1 = 0;
-      CHECK_OK(row.GetInt32(1, &c1));
+      ASSERT_OK(row.GetInt32(1, &c1));
 
       switch (pattern) {
         case C1_MATCHES_INDEX:
-          CHECK_EQ(row_idx, c1);
+          ASSERT_EQ(row_idx, c1);
           break;
         case C1_IS_DEADBEEF:
-          CHECK_EQ(0xdeadbeef, c1);
+          ASSERT_EQ(0xdeadbeef, c1);
           break;
         default:
           LOG(FATAL);
       }
     }
   }
-  CHECK_EQ(verified, num_rows);
+  ASSERT_EQ(verified, num_rows);
 }
 
 Status AlterTableTest::CreateTable(const string& table_name,
@@ -753,7 +755,7 @@ TEST_F(AlterTableTest, TestDropAndAddNewColumn) {
   InsertRows(0, kNumRows);
 
   LOG(INFO) << "Verifying initial pattern";
-  VerifyRows(0, kNumRows, C1_MATCHES_INDEX);
+  NO_FATALS(VerifyRows(0, kNumRows, C1_MATCHES_INDEX));
 
   LOG(INFO) << "Dropping and adding back c1";
   unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
@@ -763,7 +765,7 @@ TEST_F(AlterTableTest, TestDropAndAddNewColumn) {
   ASSERT_OK(AddNewI32Column(kTableName, "c1", 0xdeadbeef));
 
   LOG(INFO) << "Verifying that the new default shows up";
-  VerifyRows(0, kNumRows, C1_IS_DEADBEEF);
+  NO_FATALS(VerifyRows(0, kNumRows, C1_IS_DEADBEEF));
 }
 
 // Tests that a renamed table can still be altered. This is a regression test, we used to not carry
@@ -2023,7 +2025,7 @@ TEST_F(ReplicatedAlterTableTest, TestReplicatedAlter) {
   InsertRows(0, kNumRows);
 
   LOG(INFO) << "Verifying initial pattern";
-  VerifyRows(0, kNumRows, C1_MATCHES_INDEX);
+  NO_FATALS(VerifyRows(0, kNumRows, C1_MATCHES_INDEX));
 
   LOG(INFO) << "Dropping and adding back c1";
   unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
@@ -2036,7 +2038,11 @@ TEST_F(ReplicatedAlterTableTest, TestReplicatedAlter) {
   ASSERT_FALSE(alter_in_progress);
 
   LOG(INFO) << "Verifying that the new default shows up";
-  VerifyRows(0, kNumRows, C1_IS_DEADBEEF);
+  // TODO(KUDU-791): we should be able to assert right away, but alter-table doesn't
+  // currently obey the expected consistency semantics.
+  ASSERT_EVENTUALLY([&]() {
+    NO_FATALS(VerifyRows(0, kNumRows, C1_IS_DEADBEEF));
+  });
 }
 
 } // namespace kudu