[kudu-CR] WIP: [integration tests] scan inconsistency test

[David Ribeiro Alves (Code Review)]

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* 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 split_row(schema_.NewRow());
: ASSERT_OK(split_row->SetInt32(0, 8));
: 
: unique_ptr 
table_creator(client_->NewTableCreator());
: ASSERT_OK(table_creator->table_name(table_name)
:   .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 BuildTestRow(KuduTable* table, int index) 
{
 : unique_ptr 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(6);
 : for (int i = first_row; i < num_rows + first_row; ++i) {
 :   unique_ptr 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();
 :

[kudu-CR] WIP: [integration tests] scan inconsistency test

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: WIP: [integration tests] scan inconsistency test
..


Patch Set 1:

> Maybe rename this to 'consistency-itest' or somesuch so that it
 > provides a useful home for all consistency-related integration
 > tests, scan and otherwise?

Good idea, will change to consistency-itest.

-- 
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: [integration tests] scan inconsistency test

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: WIP: [integration tests] scan inconsistency test
..


Patch Set 1:

Maybe rename this to 'consistency-itest' or somesuch so that it provides a 
useful home for all consistency-related integration tests, scan and otherwise?

-- 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: [integration tests] scan inconsistency test

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5084

Change subject: WIP: [integration tests] scan inconsistency test
..

WIP: [integration tests] scan inconsistency test

This is a small test to expose inconsistent reads when no timestamp
is propagated by the client to tablet servers.

This is in the context of KUDU-1679 Propagate timestamps for scans.

Change-Id: I88e0d4e244be8abcef18e0b8317bacfa9bf272cb
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/consistent-scan-test.cc
2 files changed, 258 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/5084/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I88e0d4e244be8abcef18e0b8317bacfa9bf272cb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin