Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
......................................................................


Patch Set 16:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc@117
PS15, Line 117:   void ScannerThread(KuduClient::ReplicaSelection selection,
              :                      int rows_to_insert,
              :                      int first_row,
> how about we make this a fast/slow test so that we get a bit more coverage
Done


http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc@131
PS15, Line 131:       int expected_count = rows_to_insert * (i + 1);
> warning: either cast from 'int' to 'size_t' (aka 'unsigned long') is ineffe
Done


http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc@132
PS15, Line 132: // Perform a bunch of READ_YOUR_WRITES scans to all the replicas
              :       // that count the rows. And verify that the count of the 
rows
              :       // never go down from what previously observed, to ensure 
subsequent
              :       // reads will not "go back in time" regarding writes that 
other
              :       // clients have done.
              :       f
> maybe add an explanation of why we're doing this and how this verified RYR,
Done


http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc@776
PS15, Line 776:   size_t row_count;
              :   ASSERT_OK(GetRowCount(table.get(), 
KuduScanner::READ_YOUR_WRITES,
              :                         sel, 0, &row_count));
              :   EXPECT_EQ(rows_to_insert, row_count);
              :
> nit maybe move this to L745?
Done


http://gerrit.cloudera.org:8080/#/c/8823/15/src/kudu/integration-tests/consistency-itest.cc@790
PS15, Line 790: class ScanYourWritesMultiClientsParamTest :
              :     public ConsistencyITest,
              :     public ::testing::WithParamInterface<KuduClie
> maybe explain summarily the general strategy of the test?
Done



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <arod...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 24 Feb 2018 03:06:51 +0000
Gerrit-HasComments: Yes

Reply via email to