[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-21 Thread Hao Hao (Code Review)
Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Reviewed-on: http://gerrit.cloudera.org:8080/8804
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
Reviewed-by: David Ribeiro Alves 
Reviewed-by: Alexey Serbin 
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 326 insertions(+), 98 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, but someone else must approve
  David Ribeiro Alves: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 19
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-21 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 18: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc@a1726
PS16, Line 1726:
> Since ScannerOpenWhenServerShutsDownParamTest should also cover the scenari
Ah, I see.  Thanks for the explanation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 19:08:43 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-21 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 18: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 18:37:43 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-21 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 18: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 08:01:41 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 18:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163:
> Thanks for the explanation. Let's add that explanation in the comment here,
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163:
> Maybe we should just add that explanation to the docs?
Updated.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 05:23:20 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 326 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/18
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> Maybe we should just add that explanation to the docs?
Thanks for the explanation. Let's add that explanation in the comment here, 
maybe something like:

  // fall into that category because calling 
clock->Update(propagated_timestamp) verifies that propagated_timestamp + 
FLAGS_max_clock_sync_error_usec is not too far into the future.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 17
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:36:01 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> As we update the clock based on propagated timestamp at L2140, if (propagat
Maybe we should just add that explanation to the docs?
Also: obvious while we have these things in context, but likely worth 
mentioning for the documentation value is that "clean" timestamp is, by 
definition in the past.
Suggestion, something like:
There is no need to validate if the chosen timestamp is too far in the future, 
as :
- MVCC's 'clean' ts is by definition in the past (it's maximally bounded by 
safe time).
- "propagated" ts was used to update the clock above and the update would have 
returned an error if the the timestamp was too far in the future.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:42:13 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 16:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc@a1726
PS16, Line 1726:
> What happened to this test?  Maybe I'm missing something, but it seems this
Since ScannerOpenWhenServerShutsDownParamTest should also cover the scenario of 
this test case. I removed it as it is duplicated.


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@157
PS16, Line 157: TimeStamp
> nit: in the code around, we have Timestamp as a type; maybe, name it consis
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@161
PS16, Line 161: PickAndVerifyTimeStamp
> nit: ditto, PickAndVerifyTimestamp(...) ?
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@162
PS16, Line 162: tablet::TabletReplica*
> Could it be const tablet::TabletReplica& ?
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@1757
PS16, Line 1757:   case READ_YOUR_WRITES:
> nit: per coding standard, add // fallthrough
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2042
PS16, Line 2042: case READ_AT_SNAPSHOT:
> nit: per coding standard, add // fallthrough
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2117
PS16, Line 2117: .
> s/./,/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: initiated
> s/initiated with/default-constructed to/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: Since
> s/Since/since/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> is that guaranteed by something?
As we update the clock based on propagated timestamp at L2140, if (propagation 
ts + 1) is a timestamp too far in the future, which essentially means 
(propagation TS + 1 > propagation TS + FLAGS_max_clock_sync_error_usec).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:50:48 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 322 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/17
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 17
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-20 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 16:

(4 comments)

lgtm, just a nit and one last question

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2117
PS16, Line 2117: .
s/./,/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: initiated
s/initiated with/default-constructed to/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: Since
s/Since/since/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
is that guaranteed by something?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:47:44 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 16:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc@a1726
PS16, Line 1726:
What happened to this test?  Maybe I'm missing something, but it seems this 
test should stay relevant regardless of the new scan mode.


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@157
PS16, Line 157: TimeStamp
nit: in the code around, we have Timestamp as a type; maybe, name it 
consistently with that so it's ValidateTimestamp(...) ?


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@161
PS16, Line 161: PickAndVerifyTimeStamp
nit: ditto, PickAndVerifyTimestamp(...) ?


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@162
PS16, Line 162: tablet::TabletReplica*
Could it be const tablet::TabletReplica& ?

Or, maybe, Tablet* ?


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@1757
PS16, Line 1757:   case READ_YOUR_WRITES:
nit: per coding standard, add // fallthrough


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2042
PS16, Line 2042: case READ_AT_SNAPSHOT:
nit: per coding standard, add // fallthrough



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 17 Feb 2018 01:44:55 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 16: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 17 Feb 2018 00:36:50 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 321 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/16
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 14:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.h@156
PS14, Line 156:   // as such timestamp is invalid.
> nit: "such a"
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2114
PS12, Line 2114:   }
   :
> Good question/point. I wrote that and it was cryptic to me too. I now reali
I see, thanks David for the explanation.


http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc@2046
PS14, Line 2046: return Status::NotSupported("Unsupported snapshot scan mode 
specified");
> nit: I think this can FATAL out. it's us (not the user) that call this and
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 23:26:23 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 321 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/15
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 15
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2114
PS12, Line 2114:   // Note: if 'max_allowed_ts' is not obtained from 
clock_->GetGlobalLatest() it's guaranteed
   :   // to be higher than 'tmp_snap_timestamp'.
> Good question. TBH, this is refactored from previous code, so I am not sure
Good question/point. I wrote that and it was cryptic to me too. I now realize 
that I'm just referring to the fact that if we don't set a timestamp at all, it 
get's set to MAX_LONG - 1 (kInvalidTimestamp). The point is to put reader at 
ease that even if we don't get a global latest ts from the clock, it's still 
safe to compare the snap timestamp below.

Thanks for fixing this Hao



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 22:50:13 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc@2166
PS14, Line 2166: RETURN_NOT_OK(ValidateTimeStamp(tmp_snap_timestamp));
> What is this really checking. We need to validate the timestamp for READ_AT
Right, think a bit more, I do not see a case where this validation could fail 
(which essentially mean: propagation TS + 1 > propagation TS + 
FLAGS_max_clock_sync_error_usec)

So it may not worth checking here. I will remove.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 22:44:40 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 14:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.h@156
PS14, Line 156:   // as such timestamp is invalid.
nit: "such a"


http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc@2046
PS14, Line 2046: return Status::NotSupported("Unsupported snapshot scan mode 
specified");
nit: I think this can FATAL out. it's us (not the user) that call this and we 
should only call it for "snapshotty" scans


http://gerrit.cloudera.org:8080/#/c/8804/14/src/kudu/tserver/tablet_service.cc@2166
PS14, Line 2166: RETURN_NOT_OK(ValidateTimeStamp(tmp_snap_timestamp));
What is this really checking. We need to validate the timestamp for 
READ_AT_SNAPSHOT as the user can send a timestamp that is arbitrarily in the 
future, but here it can only be either "clean_timestamp" (which is valid) or 
propagated timestamp, which is necessarily valid since we've udpated the clock 
with it in line 2139.
In other words do you think that there is a test case you can write that would 
fail this check?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 20:08:57 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-16 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 14: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 18:58:54 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-15 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 320 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/14
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-15 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/13/src/kudu/tserver/tablet_server-test-base.cc
File src/kudu/tserver/tablet_server-test-base.cc:

http://gerrit.cloudera.org:8080/#/c/8804/13/src/kudu/tserver/tablet_server-test-base.cc@423
PS13, Line 423:   ScanRequestPB req;
> warning: parameter 'read_mode' is unused [misc-unused-parameters]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 16 Feb 2018 00:00:24 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-14 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 353 insertions(+), 107 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/13
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-14 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 12:

(23 comments)

http://gerrit.cloudera.org:8080/#/c/8804/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8804/12//COMMIT_MSG@15
PS12, Line 15:  choose the newest timestamp
 : within the staleness bound that allows execution of the reads 
without
 : being blocked by the in-flight transactions
> might be worth adding "if possible"
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@236
PS12, Line 236: subject to the propagated timestamp bound
> This wording is pretty hard to a casual reader (or even me) to understand.
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@237
PS12, Line 237: newest timestamp within
> latest timestamp higher than
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@242
PS12, Line 242: the timestamp
> a timestamp
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@244
PS12, Line 244: should use it
> Is this something that end-users have to worry about, or can we word this s
Updated.

Currently, we use current clock time of the server as the propagation time. 
This would bump up the propagation time unnecessarily for the next read in this 
mode.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@245
PS12, Line 245: wait
> nit: waiting
Done


http://gerrit.cloudera.org:8080/#/c/8804/11/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/11/src/kudu/tserver/tablet_server-test.cc@185
PS11, Line 185:   void ScanYourWritesTest(uint64_t propagated_timestamp, 
ScanResponsePB* resp);
> warning: parameter 'propagated_timestamp' is const-qualified in the functio
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

PS12:
> We are missing a C++ client test in this patch. Is that excluded on purpose
C++ client test in the following CR https://gerrit.cloudera.org/#/c/8823/


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TEST_F(TabletServerTest, TestScanYourWrites) {
> nit: add a comment at the top of this test describing what the test does
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TestScanYourWrites
> It worries me a bit that most tests of this functionality overwhelmingly te
Added more integration tests in C++ client which would definitely fail with 
READ_LATEST.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TestScanYourWrites
> I agree that a deterministic test would be nice and I think it should be po
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1927
PS12, Line 1927: e
> nit: missing period
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1958
PS12, Line 1958: TEST_F(TabletServerTest, 
TestScanYourWrites_WithoutPropagatedTimestamp) {
> nit: add test comment
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1960
PS12, Line 1960: perform a write
> nit: use capitalization and punctuation for code comments per https://googl
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1972
PS12, Line 1972: perform a write
> punctuation
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h@155
PS12, Line 155:   // it exceeds the maximum allowed clock synchronization error 
time.
> Can we add a note about the context, i.e. why this matters?
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h@158
PS12, Line 158: if
> s/if/that/
Done


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2113
PS12, Line 2113:
> add: DCHECK(s.ok()) ?
I think even for status that returns 'NotSupported' can reach here.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2114
PS12, Line 2114:   // Note: if 'max_allowed_ts' is not obtained from 
clock_->GetGlobalLatest() it's guaranteed
   :   // to be higher than 'tmp_snap_timestamp'.
> I'm having trouble following this:
Good question. TBH, this is refactored from previous code, so I am not sure 
about 

[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-13 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 12:

(20 comments)

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@236
PS12, Line 236: subject to the propagated timestamp bound
This wording is pretty hard to a casual reader (or even me) to understand.

I think it would be more clear to say something like: Each tablet server will 
choose a timestamp to use for a server-local snapshot scan subject to the 
following criteria: (1) It will be higher than the propagated timestamp, (2) It 
will attempt to minimize latency caused by waiting for outstanding write 
transactions to complete.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@237
PS12, Line 237: newest timestamp within
latest timestamp higher than


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@242
PS12, Line 242: the timestamp
a timestamp


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@244
PS12, Line 244: should use it
Is this something that end-users have to worry about, or can we word this so 
that it sounds like Kudu will take care of it? If the latter, how about

"The Kudu client library will use it as the propagated timestamp for subsequent 
reads" or something along those lines? However I'm not sure why we are making 
the distinction here between a normally propagated timestamp and this different 
thing.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/common/common.proto@245
PS12, Line 245: wait
nit: waiting


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

PS12:
We are missing a C++ client test in this patch. Is that excluded on purpose?


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TestScanYourWrites
> It worries me a bit that most tests of this functionality overwhelmingly te
I agree that a deterministic test would be nice and I think it should be 
possible using two clients.

It would also be nice to have a non-deterministic test where you have two 
clients in RYW mode reading and scanning. Loop them concurrently in separate 
threads at least a certain number of rounds or amount of time, as well as until 
both of the following conditions hold, while also triggering leader elections 
to make it easier to hit anomalies:

1. neither has ever violated RYW locally
2. we have observed anomalies where if we had been using a strict serializable 
approach then certain writes would have appeared, but they don't because of the 
hidden channel (the other client).


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TEST_F(TabletServerTest, TestScanYourWrites) {
nit: add a comment at the top of this test describing what the test does


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1927
PS12, Line 1927: e
nit: missing period


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1958
PS12, Line 1958: TEST_F(TabletServerTest, 
TestScanYourWrites_WithoutPropagatedTimestamp) {
nit: add test comment


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1960
PS12, Line 1960: perform a write
nit: use capitalization and punctuation for code comments per 
https://google.github.io/styleguide/cppguide.html#Punctuation,_Spelling_and_Grammar


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1972
PS12, Line 1972: perform a write
punctuation


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h@155
PS12, Line 155:   // it exceeds the maximum allowed clock synchronization error 
time.
Can we add a note about the context, i.e. why this matters?

nit: Also perhaps rename to ValidateSnapshotTimestamp() ? Usually Stamp in 
Timestamp is not capitalized in the Kudu code, and this seems specific to 
snapshot timestamps.


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.h@158
PS12, Line 158: if
s/if/that/


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2113
PS12, Line 2113:
add: DCHECK(s.ok()) ?


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_service.cc@2114
PS12, Line 2114:   // Note: if 'max_allowed_ts' is not obtained from 
clock_->GetGlobalLatest() it's guaranteed
   :   // to be higher than 

[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-09 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8804/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8804/12//COMMIT_MSG@15
PS12, Line 15:  choose the newest timestamp
 : within the staleness bound that allows execution of the reads 
without
 : being blocked by the in-flight transactions
might be worth adding "if possible"


http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/12/src/kudu/tserver/tablet_server-test.cc@1924
PS12, Line 1924: TestScanYourWrites
It worries me a bit that most tests of this functionality overwhelmingly tests 
the "happy" case.  A good test of that is: if you changed the mode of all tests 
here to READ_LATEST would it fail? if not that's ok as long as we have some 
test somewhere that would, however I didn't see such a tests even after the 
client implementations.

Jepsen tests are good to cover more possibilities, but some deterministic tests 
just for RYW should exist.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 09 Feb 2018 20:09:16 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-08 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions. READ_YOUR_WRITES mode is not
repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 234 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/12
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-07 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 11:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@222
PS10, Line 222: tion is
> nit: snapshot timestamp
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@228
PS10, Line 228:
> nit: choose
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@229
PS10, Line 229:  serv
> nit: such that
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@231
PS10, Line 231:
> The 'stale' terminology hasn't been introduced here, so I think it would be
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@233
PS10, Line 233:
> I think it would be better to say 'different results' here in order to avoi
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1922
PS10, Line 1922:
> We decided we also wanted scan your reads right? is that covered?
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1944
PS10, Line 1944:   ASSERT_EQ(R"((in
> I know this is likely copy/paste but remove (adds nothing)
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953
PS10, Line 1953:   ASSERT_EQ(kNumRows, results.size());
   :   ASSERT_EQ(R"((int32 key=0, int32 int_val
> I think we discussed a couple of reasons why it might be interesting to hav
Right, updated it accordingly.


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1989
PS10, Line 1989: Hybrid
> same
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.h@158
PS10, Line 158: ing to the scan m
> on the read mode?
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2107
PS10, Line 2107: us s = server_->cl
> nit add a predict false
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2161
PS10, Line 2161: )
> use kMinTimestamp or something? I understand you can't use kInvalidTimestam
Done


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2163
PS10, Line 2163: Timestamp(std::max(propagated_timestamp + 1, clean_timestamp));
   : RETURN_NOT_OK(ValidateTimeStamp(tmp_snap_timestamp));
> std:max ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Thu, 08 Feb 2018 04:36:00 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-07 Thread Hao Hao (Code Review)
Hello Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, Kudu 
Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions. READ_YOUR_WRITES mode is not
repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 234 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/11
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-06 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953
PS10, Line 1953:   // Make sure that there is no snapshot timestamp sent back.
   :   ASSERT_TRUE(!resp.has_snap_timestamp());
> I see, so looks like you are proposing to make this read mode fault-toleran
I think we discussed a couple of reasons why it might be interesting to have 
the response include the timestamp (like using that to update the last 
propagated timestamp). I agree that fault-tolerance work doesn't need to be 
included in this patch though



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 06 Feb 2018 23:50:05 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-05 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953
PS10, Line 1953:   // Make sure that there is no snapshot timestamp sent back.
   :   ASSERT_TRUE(!resp.has_snap_timestamp());
> I'm still unsure why we wouldn't the response to include the timestamp. I a
I see, so looks like you are proposing to make this read mode fault-tolerant as 
well. Which means we need to make it an ordered scan and ensure the client sent 
out the last primary key to retry on.

I do not see any reasons to not make this mode fault tolerant, but I do not see 
the reasons why we have to return the chosen snapshot timestamp for the scan to 
retry on another server in this case. I think the next tsever that the scan is 
retried on can chose the timestamp freely based on the conditions, right?

Moreover, would you mind I do another patch if we do want RYW to be fault 
tolerant, to make it more clear?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:21:14 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-02 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@237
PS10, Line 237:   READ_YOUR_WRITES = 3;
> It may be good to add docs about whether a snapshot timestamp and propagate
Will update.

We do not return the chosen snapshot timestamp, because as we discussed 
offline, for multi tablet scan we should not reuse the timestamps across 
servers in this mode. Each server should be free to choose a timestamp as long 
as it respects the aforementioned condition (This is documented in the design 
doc as well :)). The returned propagated timestamp is set to 'Now' of the 
tserver (same as other scan mode). However, as discussed the client should only 
update the propagated timestamp after the scan of last tablet for multi tablet 
scan.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 02 Feb 2018 22:49:01 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-02 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@222
PS10, Line 222:  snapshot
nit: snapshot timestamp


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@228
PS10, Line 228: take
nit: choose


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@229
PS10, Line 229: which
nit: such that


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1922
PS10, Line 1922: TestScanYourWrites
We decided we also wanted scan your reads right? is that covered?


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1944
PS10, Line 1944:   // Send the call
I know this is likely copy/paste but remove (adds nothing)


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953
PS10, Line 1953:   // Make sure that there is no snapshot timestamp sent back.
   :   ASSERT_TRUE(!resp.has_snap_timestamp());
I'm still unsure why we wouldn't the response to include the timestamp. I agree 
that the scan itself (seen as a collection os scans to multiple tablets) can't 
be described by a single timestamp, but having the timestamp in the response 
would allows to have fault-tolerant scans, right?


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1989
PS10, Line 1989:   // Send the call
same


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.h@158
PS10, Line 158: on the read mode,
on the read mode?


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2107
PS10, Line 2107: s.IsNotSupported()
nit add a predict false


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2161
PS10, Line 2161: 0
use kMinTimestamp or something? I understand you can't use kInvalidTimestamp 
(it's big) but don't want to accidentally set time to 0 (been there done that 
:))


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_service.cc@2163
PS10, Line 2163: tmp_snap_timestamp > clean_timestamp ?
   :  tmp_snap_timestamp : clean_timestamp;
std:max ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 02 Feb 2018 22:33:39 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-02 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@231
PS10, Line 231: stale
The 'stale' terminology hasn't been introduced here, so I think it would be 
best to avoid it.  Perhaps 'two READ_YOUR_WRITES scans, ...'


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@233
PS10, Line 233: inconsistent
I think it would be better to say 'different results' here in order to avoid 
confusion over the formal consistency guarantees.


http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/common/common.proto@237
PS10, Line 237:   READ_YOUR_WRITES = 3;
It may be good to add docs about whether a snapshot timestamp and propagated 
timestamp are returned, and what the client should do with them.

I'm surprised to see in the latest revision that READ_YOUR_WRITES doesn't 
return a snapshot timestamp, why is that?  And is the returned propagated 
timestamp set to the snapshot timestamp of the scan?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 02 Feb 2018 17:17:09 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-01 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@227
PS7, Line 227: som
> might be more clear to replace 'any' with 'some' here.
Done


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@227
PS7, Line 227:
> in
Done


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@231
PS7, Line 231: Reads in this mode are not repeatable: two stale reads, even if 
they
 :   // provide the same propaga
> I think it's worth going into a little more detail - READ_YOUR_WRITES is by
Hmm, I actually updated this in the latest patch (patch 9). Sorry for any 
confusion.


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@233
PS7, Line 233: mps an
> s/picked/chosen
Done


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.h@154
PS7, Line 154: so far in the future that
 :   // it exce
> 'not so far in the future that it exceeds'
Done


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.cc@1625
PS7, Line 1625: t
> not your fault, but this looks like an extraneous quote.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Thu, 01 Feb 2018 22:45:10 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-01 Thread Hao Hao (Code Review)
Hello Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, Kudu 
Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions. READ_OWN_WRITES mode is not
repeatable. However, it allows client local read-your-writes.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 260 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/10
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-01 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 7:

(6 comments)

Just a few nits on docs, but otherwise LGTM.

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@227
PS7, Line 227: n
in


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@227
PS7, Line 227: any
might be more clear to replace 'any' with 'some' here.


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@231
PS7, Line 231: READ_YOUR_WRITES is repeatable, i.e. all future reads at the 
same timestamp
 :   // will yield the same rows
I think it's worth going into a little more detail - READ_YOUR_WRITES is by 
itself not repeatable, but a READ_YOUR_WRITES followed by a READ_AT_SNAPSHOT 
with the same timestamp chosen is repeatable.


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/common/common.proto@233
PS7, Line 233: picked
s/picked/chosen


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.h@154
PS7, Line 154: too far in the future that
 :   // exceeds
'not so far in the future that it exceeds'


http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/7/src/kudu/tserver/tablet_service.cc@1625
PS7, Line 1625: '
not your fault, but this looks like an extraneous quote.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Thu, 01 Feb 2018 19:41:45 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-01 Thread Hao Hao (Code Review)
Hello Alexey Serbin, Mike Percy, David Ribeiro Alves, Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions. READ_OWN_WRITES mode is not
repeatable. However, it allows client local read-your-writes.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 260 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/9
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-01-31 Thread Hao Hao (Code Review)
Hello Alexey Serbin, Mike Percy, David Ribeiro Alves, Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

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

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

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions. READ_OWN_WRITES mode is not
repeatable. However, it allows client local read-your-writes.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 260 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/8804/8
--
To view, visit http://gerrit.cloudera.org:8080/8804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy