You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2019/03/15 20:56:44 UTC

[kudu-CR] update outdated comments for Read-your-writes scan

Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12768


Change subject: update outdated comments for Read-your-writes scan
......................................................................

update outdated comments for Read-your-writes scan

Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
---
M src/kudu/client/client.h
1 file changed, 3 insertions(+), 33 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: update outdated comments for Read-your-writes scan
......................................................................

update outdated comments for Read-your-writes scan

Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
---
M src/kudu/client/client.h
1 file changed, 3 insertions(+), 35 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Alexey should also take a look, since he wrote the original text.

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: While it can be used to get Read-Your-Writes consistency,
             :   /// it is recommended to use READ_YOUR_WRITES scan mode directly. See
             :   /// KuduScanner for more details.
Do we even want to retain this snippet? Under what circumstances would you want to RYW the "old" way?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 15 Mar 2019 21:27:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: While it can be used to get Read-Your-Writes consistency,
             :   /// it is recommended to use READ_YOUR_WRITES scan mode directly. See
             :   /// KuduScanner for more details.
> I don't see any, but keep it in case existing users if this pattern is curi
The issue is that we've taken away all the information that'd help someone use GetLatestObservedTimestamp() into the old RYW approach.

If anything, I think we should link this method to the CLIENT_PROPAGATED ExternalConsistencyMode, because that's actually useful to know.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 15 Mar 2019 23:16:31 +0000
Gerrit-HasComments: Yes

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: n::CLIENT_PROPAGATED external consistency mode.
             :   ///
             :   /// @note This method is experime
> If we are talking about RYW already implemented by the client library itsel
I am not sure if we should remove this method which can be used for ExternalConsistencyMode as Adar pointed out.


http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: n::CLIENT_PROPAGATED external consistency mode.
             :   ///
             :   /// @note This method is experime
> The issue is that we've taken away all the information that'd help someone 
Sounds good, updated.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 19 Mar 2019 19:01:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: While it can be used to get Read-Your-Writes consistency,
             :   /// it is recommended to use READ_YOUR_WRITES scan mode directly. See
             :   /// KuduScanner for more details.
> Do we even want to retain this snippet? Under what circumstances would you 
I don't see any, but keep it in case existing users if this pattern is curious?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 15 Mar 2019 22:23:58 +0000
Gerrit-HasComments: Yes

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................

update outdated comments for Read-your-writes scan

Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Reviewed-on: http://gerrit.cloudera.org:8080/12768
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/client.h
1 file changed, 3 insertions(+), 35 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 19 Mar 2019 19:15:20 +0000
Gerrit-HasComments: No

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 20 Mar 2019 03:47:46 +0000
Gerrit-HasComments: No

[kudu-CR] update outdated comments for Read-your-writes scan

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12768 )

Change subject: update outdated comments for Read-your-writes scan
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/12768/1/src/kudu/client/client.h@502
PS1, Line 502: While it can be used to get Read-Your-Writes consistency,
             :   /// it is recommended to use READ_YOUR_WRITES scan mode directly. See
             :   /// KuduScanner for more details.
> The issue is that we've taken away all the information that'd help someone 
If we are talking about RYW already implemented by the client library itself, maybe we are ready to remove this method from the public API?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5eaa8c64edcb35a6936d8699396d45ef8457dea
Gerrit-Change-Number: 12768
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 16 Mar 2019 00:32:45 +0000
Gerrit-HasComments: Yes