Alexey Serbin has posted comments on this change.

Change subject: [i-tests] scan token timestamp propagation test
......................................................................


Patch Set 1:

(2 comments)

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

Line 591:     ASSERT_OK(builder.SetSnapshotRaw(ts));
> maybe I'm missing something: if the scan tokens are propagating timestamps 
Yes, it seems I see your point.  Right, that's the essence of the timestamp 
propagation.

May be, I misunderstood Dan's comment -- I thought Dan asked whether the 
timestamp for the token-to-be is assigned implicitly from the client's latest 
observed one, i.e. whether the generated token would have the timestamp set 
even if we didn't set it explicitly to any value.  In my comment I addressed 
that thing.  That's because I saw some sort of parallel in here: check 
scanner-internal.cc, line 423. 

OK, it seems it's just my misunderstanding.  I'll remove the timestamp 
assignment above.


PS1, Line 606: shared_ptr<KuduClient> client;
             :     ASSERT_OK(cluster_->CreateClient(nullptr, &client));
             :     shared_ptr<KuduTable> table;
             :     ASSERT_OK(client->OpenTable(table_name_, &table));
             :     KuduScanner* scanner_raw;
             :     
ASSERT_OK(KuduScanToken::DeserializeIntoScanner(client.get(), scan_token,
             :                                                     
&scanner_raw));
             :     unique_ptr<KuduScanner> scanner(scanner_raw);
             :     scanner->SetTimeoutMillis(scan_timeout_msec);
             :     ASSERT_OK(scanner->Open());
             :     size_t row_num = 0;
             :     while (scanner->HasMoreRows()) {
             :       KuduScanBatch batch;
             :       ASSERT_OK(scanner->NextBatch(&batch));
             :       row_num += batch.NumRows();
             :     }
             :     EXPECT_EQ(0, row_num);
> the trickyness stems from the fact that you could get timeouts that are unr
ok, I see; that makes sense.  I could not see other way of checking that.  Will 
need to explore other alternatives around.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
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: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-HasComments: Yes

Reply via email to