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 <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Comment-Date: Thu, 08 Feb 2018 04:36:00 +0000
Gerrit-HasComments: Yes

Reply via email to