David Ribeiro Alves has posted comments on this change. Change subject: WIP: [integration tests] scan inconsistency test ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/5084/1/src/kudu/integration-tests/consistent-scan-test.cc File src/kudu/integration-tests/consistent-scan-test.cc: PS1, Line 48: class ConsistentScanTest : public KuduTest { It would be great to have a way to have each specific test start a cluster a certain way. For certain tests multiple tablets of one replica are ideal, for other we need a single tablet with multiple replicas. A helper method like StartCluster(int num_tablets, int num_replicas) would be helpful. PS1, Line 48: ConsistentScanTest Like we had discussed yesterday and like Todd suggested ConsistencyITest would be a better name for the class/test. PS1, Line 80: ExternalMiniClusterOptions In most of the cases I can think of we need a MiniCluster, not an ExternalMIniCluster, so that we can skew the clock. Maybe worth pondering if we'll need a test that stops/kills servers in which case having the option to choose between the two would be best. PS1, Line 84: // Creates a table with the specified name and replication factor. : void CreateTable(const string& table_name, int num_replicas, : shared_ptr<KuduTable>* table) { : // Using range partitions with high split value for the key -- this is : // to keep the contents of the table primarily at one tablet server. : unique_ptr<KuduPartialRow> split_row(schema_.NewRow()); : ASSERT_OK(split_row->SetInt32(0, 8)); : : unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); : ASSERT_OK(table_creator->table_name(table_name) : .schema(&schema_) : .add_range_partition_split(split_row.release()) : .set_range_partition_columns({ key_column_name_ }) : .num_replicas(num_replicas) : .Create()); : ASSERT_OK(client_->OpenTable(table_name, table)); : } Worth looking into other itests to see if we can reuse some of the setup/helpers there. TabletHistoryGcITest or RaftConsensusITest come to mind PS1, Line 102: unique_ptr<KuduInsert> BuildTestRow(KuduTable* table, int index) { : unique_ptr<KuduInsert> insert(table->NewInsert()); : KuduPartialRow* row = insert->mutable_row(); : CHECK_OK(row->SetInt32(0, index)); : CHECK_OK(row->SetInt32(1, index * 2)); : CHECK_OK(row->SetStringCopy(2, Slice(StringPrintf("hello %d", index)))); : return insert; : } : : // Inserts given number of tests rows into the default test table : // in the context of the specified session. : Status InsertTestRows(KuduSession* session, InsertFlushOptions flush_opt, : int num_rows, int first_row = 0) { : RETURN_NOT_OK(session->SetFlushMode(KuduSession::AUTO_FLUSH_BACKGROUND)); : session->SetTimeoutMillis(60000); : for (int i = first_row; i < num_rows + first_row; ++i) { : unique_ptr<KuduInsert> insert(BuildTestRow(table_.get(), i)); : RETURN_NOT_OK(session->Apply(insert.release())); : } : if (flush_opt == OPT_FLUSH) { : RETURN_NOT_OK(session->Flush()); : } : return Status::OK(); : } : : Status GetRowCount(KuduClient::ReplicaSelection replica_selection, : KuduScanner::ReadMode read_mode, : size_t* row_count) { : KuduScanner scanner(table_.get()); : RETURN_NOT_OK(scanner.SetReadMode(read_mode)); : RETURN_NOT_OK(scanner.SetSelection(replica_selection)); : // KUDU-1656: there might be timeouts, so re-try the operations to : // avoid test flakiness. : Status row_count_status; : size_t actual_row_count = 0; : for (size_t i = 0; i < 3; ++i) { : row_count_status = scanner.Open(); : if (!row_count_status.ok()) { : if (row_count_status.IsTimedOut()) { : // Start the row count over again. : continue; : } : RETURN_NOT_OK(row_count_status); : } : size_t count = 0; : while (scanner.HasMoreRows()) { : KuduScanBatch batch; : row_count_status = scanner.NextBatch(&batch); : if (!row_count_status.ok()) { : if (row_count_status.IsTimedOut()) { : // Break the NextBatch() cycle and start row count over again. : break; : } : RETURN_NOT_OK(row_count_status); : } : count += batch.NumRows(); : } : if (row_count_status.ok()) { : // Success: stop the retry cycle. : actual_row_count = count; : break; : } : } : RETURN_NOT_OK(row_count_status); : if (row_count) { : *row_count = actual_row_count; : } : return Status::OK(); : } same PS1, Line 193: TEST_F(ConsistentScanTest, DISABLED_TwoReadsAfterWrite) { I think the cleanest to test scan response timestamp propagation is to do the following: - Start two mini cluster tservers and create a table with two tablets ({a, b} order matters), single replica. - Set the flag to use the mock wall clock. - Advance the clock in tablet a's tserver by some amount - Write a row to tablet a, discard the client. - With a new client, read the row from tablet a (snapshot read, no assigned timestamp), then write a row to tablet b. Take note of write timestamp. Discard the client. Now Scan both tablets at the timestamp of the write to tablet b. Since b followed a (quite literally, we scanned a before writing b). We should see both rows, but because we didn't propagate the timestamp from a's scan to b's write that shouldn't be the case. -- To view, visit http://gerrit.cloudera.org:8080/5084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88e0d4e244be8abcef18e0b8317bacfa9bf272cb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
