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/30 00:13:24 UTC

[kudu-CR] [i-tests] test for timestamp propagation with write ops

Alexey Serbin has uploaded a new change for review.

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

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................

[i-tests] test for timestamp propagation with write ops

Added an integration test to verify that C++ client propagates
timestamp with write operations.

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:
  https://issues.apache.org/jira/browse/KUDU-1770

Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 55 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [i-tests] test for timestamp propagation with write ops

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 1:

Mike: We've been pushing the fix and the test separately for this stuff. I think it makes sense as most of these things are tricky and we need to make sure that the test fails without the fix. That way its easy to verify this during review or later if needed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1770 [i-tests] test for timestamp propagation with write ops

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [i-tests] test for timestamp propagation with write ops

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 1:

(1 comment)

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

PS1, Line 403: // If the client propagates the timestamp with write operations,
             :   // the timestamp received from the second server should be greater
             :   // than the timestamp received from the first server.
             :   EXPECT_GT(ts, ts_ref);
can you add a scan at ts+1 and make sure you see both rows?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-1770 [i-tests] test for timestamp propagation with write ops

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/5268

to look at the new patch set (#2).

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................

KUDU-1770 [i-tests] test for timestamp propagation with write ops

Added an integration test to verify that C++ client propagates
timestamp with write operations.

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-1770 C++ client: propagate timestamp for write operations

Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 80 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/5268/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5268
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1770 [i-tests] test for timestamp propagation with write ops

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/5268

to look at the new patch set (#4).

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................

KUDU-1770 [i-tests] test for timestamp propagation with write ops

Added an integration test to verify that C++ client propagates
timestamp with write operations.

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-1770 C++ client: propagate timestamp for write operations

Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 80 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/5268/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5268
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1770 [i-tests] test for timestamp propagation with write ops

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1770 [i-tests] test for timestamp propagation with write ops

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/5268

to look at the new patch set (#3).

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................

KUDU-1770 [i-tests] test for timestamp propagation with write ops

Added an integration test to verify that C++ client propagates
timestamp with write operations.

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-1770 C++ client: propagate timestamp for write operations

Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 80 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/5268/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5268
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [i-tests] test for timestamp propagation with write ops

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 1:

(3 comments)

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

PS1, Line 360: the
> s/the //
Done


PS1, Line 379: at
> missing a word
Done


PS1, Line 403: // If the client propagates the timestamp with write operations,
             :   // the timestamp received from the second server should be greater
             :   // than the timestamp received from the first server.
             :   EXPECT_GT(ts, ts_ref);
> can you add a scan at ts+1 and make sure you see both rows?
Good idea -- will do.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1770 [i-tests] test for timestamp propagation with write ops

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.

Change subject: KUDU-1770 [i-tests] test for timestamp propagation with write ops
......................................................................


KUDU-1770 [i-tests] test for timestamp propagation with write ops

Added an integration test to verify that C++ client propagates
timestamp with write operations.

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-1770 C++ client: propagate timestamp for write operations

Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Reviewed-on: http://gerrit.cloudera.org:8080/5268
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/consistency-itest.cc
1 file changed, 80 insertions(+), 0 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
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: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [i-tests] test for timestamp propagation with write ops

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 1:

(2 comments)

Test looks good

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

PS1, Line 360: the
s/the //


PS1, Line 379: at
missing a word


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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] test for timestamp propagation with write ops

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: [i-tests] test for timestamp propagation with write ops
......................................................................


Patch Set 1:

Seems better to simply squash this commit with CR 5269 so that the test and the fix go in at the same time.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1be285fd860f0608f5d4ce3be57c4e4b04c63455
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No