You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/11/24 22:30:38 UTC
[kudu-CR] [i-tests] scan token timestamp propagation test
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5219
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
[i-tests] scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 111 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/5219/1
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5219/1/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:
PS1, Line 611: ASSERT_OK(KuduScanToken::DeserializeIntoScanner(client.get(), scan_token,
: &scanner_raw));
> how about here we assert that the lot here is 'ts' from the block above and
It could be a good solution, however the scan below does not succeed, so this would not work.
Or some other changes are needed as well?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5219/1/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:
PS1, Line 611: ASSERT_OK(KuduScanToken::DeserializeIntoScanner(client.get(), scan_token,
: &scanner_raw));
> It could be a good solution, however the scan below does not succeed, so th
ok, it seems I see what you mean: that's what we should see in case of success. If it fails, it fails and we don't do any conclusions -- that's the essence.
All right, this sounds good to me. Will update in a moment.
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/5219/1/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:
PS1, Line 397: * Discard the client object.
> why is it relevant to discard the client at the end? are you doing anything
It's not that crucial, actually. I just put it here to be in line with the rest of the list.
If it makes people wonder what it is, I will remove this step since it's not that crucial for understanding the idea behind the test.
PS1, Line 526: // The real-world use-case behind this test is as following:
: //
: // * An external entity such as Spark job writes some data into the Kudu
: // database. Then, it builds a scan token which would allow to fetch
: // some related data at the specified timestamp. The scan token is
: // serialized and output as a string.
: //
: // * Another external entity (e.g., another client application based
: // on the Kudu C++ client API) is fed with the serialized scan token
: // obtained at the previous step. It performs a scan operation built
: // from the scan token it's fed with.
> maybe be a bit more abstract? i mean no need to be talking specific scenari
Done
PS1, Line 540: , acting as an external entity,
> what does this mean?
Well, nothing particular -- all clients act as external entities.
The idea was to express that it's possible to pass scan tokens between unrelated client applications.
I'll remove this wording.
PS1, Line 541: which clock is advanced.
> missing some words/rephrase?
Done
PS1, Line 552: since the timeout for the scan operation
: // is set to much shorter time interval
> aren't you using a mock clock that doesn't advance by itself? why does the
I wanted to point that the test would not hang, but ran fast and reported an error right away due the the described facts.
PS1, Line 555: TEST_F(ConsistencyITest, DISABLED_TestScanTokenTimestampPropagation)
> this fails all the time without the fix, right?
Yes, it does fail without the fix, 100%
Line 591: ASSERT_OK(builder.SetSnapshotRaw(ts));
> Why do you have to set the raw snapshot timestamp? Shouldn't it be propaga
Currently that's not the case -- the ScanConfiguration object underneath the KuduScanTokenBuilder is not implicitly affected while building the tokens. That sort of 'automation' is available only for KuduScanner while calling OpenTablet() method (i.e. while performing a scan operation at the table).
However, you make a good point here -- may be we should make KuduScanTokenBuilder and KuduScanner similar in that regard.
David, what do you think?
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);
> I think basing the test on a timeout is tricky, is there a way to make an a
What's exactly tricky? I don't see any trickiness here -- please check my previous comment.
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-420 [i-tests] scan token timestamp propagation test
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-420 [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 5: Code-Review+2
--
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: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/5219/1/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:
PS1, Line 397: * Discard the client object.
why is it relevant to discard the client at the end? are you doing anything more after this? if so maybe say what it is?
PS1, Line 526: // The real-world use-case behind this test is as following:
: //
: // * An external entity such as Spark job writes some data into the Kudu
: // database. Then, it builds a scan token which would allow to fetch
: // some related data at the specified timestamp. The scan token is
: // serialized and output as a string.
: //
: // * Another external entity (e.g., another client application based
: // on the Kudu C++ client API) is fed with the serialized scan token
: // obtained at the previous step. It performs a scan operation built
: // from the scan token it's fed with.
maybe be a bit more abstract? i mean no need to be talking specific scenarios in the test IMO. just mention that we can get scan tokens from a client (c++/java) and use them on another client (c++/java) and that the lpt should be set properly.
PS1, Line 540: , acting as an external entity,
what does this mean?
PS1, Line 541: which clock is advanced.
missing some words/rephrase?
PS1, Line 552: since the timeout for the scan operation
: // is set to much shorter time interval
aren't you using a mock clock that doesn't advance by itself? why does the timeout matter?
PS1, Line 555: TEST_F(ConsistencyITest, DISABLED_TestScanTokenTimestampPropagation)
this fails all the time without the fix, right?
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);
I think basing the test on a timeout is tricky, is there a way to make an assertion on actual clock setting/timestamp movement?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] KUDU 420 scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5219
to look at the new patch set (#4).
Change subject: [i-tests] KUDU 420 scan token timestamp propagation test
......................................................................
[i-tests] KUDU 420 scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 107 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/5219/4
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5219
to look at the new patch set (#2).
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
[i-tests] scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 104 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/5219/2
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-420 [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5219
to look at the new patch set (#5).
Change subject: KUDU-420 [i-tests] scan token timestamp propagation test
......................................................................
KUDU-420 [i-tests] scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 107 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/5219/5
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves 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));
> Currently that's not the case -- the ScanConfiguration object underneath th
maybe I'm missing something: if the scan tokens are propagating timestamps correctly and we don't set a timestamp on the snapshot scan then the server should choose a timestamp that is higher than the last write. that is the point of timestamp propagation in scan tokens no?
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);
> What's exactly tricky? I don't see any trickiness here -- please check my
the trickyness stems from the fact that you could get timeouts that are unrelated with what you are testing and that the test is only doing an indirect check. My comment was: can you do direct check somewhere? Its cool if not possible but if it is it would be preferable
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-420 [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: KUDU-420 [i-tests] scan token timestamp propagation test
......................................................................
KUDU-420 [i-tests] scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Reviewed-on: http://gerrit.cloudera.org:8080/5219
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 107 insertions(+), 4 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5219/3//COMMIT_MSG
Commit Message:
Line 15: This is in the context of the following JIRA item:
I think it's better to simply put "KUDU-420. " at the beginning of the commit message.
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5219
to look at the new patch set (#3).
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
[i-tests] scan token timestamp propagation test
Added an integration test to verify timestamp propagation via scan
tokens for C++ client.
The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.
This is in the context of the following JIRA item:
KUDU-420 c++ client: implement HT timestamp propagation via scan tokens
Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 107 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/5219/3
--
To view, visit http://gerrit.cloudera.org:8080/5219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47cd067248f4a26c4605f075ec5ee30da71f6f30
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 3: Code-Review+2
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [i-tests] KUDU 420 scan token timestamp propagation test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [i-tests] KUDU 420 scan token timestamp propagation test
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5219/3//COMMIT_MSG
Commit Message:
Line 15: This is in the context of the following JIRA item:
> I think it's better to simply put "KUDU-420. " at the beginning of the comm
Done
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5219/1/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:
PS1, Line 611: ASSERT_OK(KuduScanToken::DeserializeIntoScanner(client.get(), scan_token,
: &scanner_raw));
how about here we assert that the lot here is 'ts' from the block above and at that the lot after each scan is higher?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [i-tests] scan token timestamp propagation test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [i-tests] scan token timestamp propagation test
......................................................................
Patch Set 1:
(1 comment)
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));
Why do you have to set the raw snapshot timestamp? Shouldn't it be propagated automatically as the latest observed?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes