You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/09/30 00:54:54 UTC
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4569
Change subject: [c++ client] added few deprecation notes
......................................................................
[c++ client] added few deprecation notes
KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp()
as experimental
Added deprecation notes for methods related to getting and setting
hybrid timestamps to perform READ_AT_SNAPSHOT scan:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 18 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/1
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4569
to look at the new patch set (#3).
Change subject: [c++ client] added few deprecation notes
......................................................................
[c++ client] added few deprecation notes
KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp()
as experimental
Added notes for methods related to getting and setting
hybrid timestamps to perform READ_AT_SNAPSHOT scan:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
David Alves is anticipating some changes there: in short, the timestamp
might be replaced some opaque type (e.g., a sequence of bytes).
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/3
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] added few deprecation notes
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG
Commit Message:
Line 18: David Alves is anticipating some changes there: in short, the timestamp
oh, also might be better to point to the JIRA (KUDU-611) that to me :)
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4569
to look at the new patch set (#6).
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
[c++ client] notes for timestamp-related methods
Added notes on anticipated changes on methods related to getting
and setting hybrid timestamps while performing READ_AT_SNAPSHOT
scan operations:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Some changes are anticipated there in the context of KUDU-611.
In short, the timestamp might be replaced with some opaque type
(e.g., a sequence of bytes).
Also, added code sample to provide an idea how to use value returned
by KuduClient::GetLatestObservedTimestamp() to set appropriate snapshot
timestamp via KuduScanner::SetSnapshotRaw().
This addresses the following JIRA issues:
KUDU-1661 Mark KuduClient::GetLatestObservedTimestamp()
as experimental
KUDU-1663 Clean-up in-code documentation for KuduScanner::ReadMode
and KuduClient::GetLatestObservedTimestamp
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 43 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/6
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] added few deprecation notes
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4569/3/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 386: /// The latest observed timestamp can be used to start a snapshot scan on a
doesn't have to be in this patch, but it would be good to note that this comment is a "workaround" for RYW and that, as you found out, the user needs to add 1 to actually get the correct timestamp back.
PS3, Line 1816: one of
nit: s/one of/a
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4569/5/src/kudu/client/client.h
File src/kudu/client/client.h:
PS5, Line 388: See
lost word.
Line 390: /// Below is an example how to use this method along with
Can you preface this by:
How to get Read-Your-Writes consistency:
<rest of the text here>
And add a note at the end that states:
"There are currently races in which, in rare occasions, Read-Your-Writes consistency might not hold even in this case. These are being taken care of as part of KUDU-430."
Line 405: /// uint64_t snapshot_timestamp = client->GetLatestObservedTimestamp();
can you change this to:
uint64_t snapshot_timestamp = client->GetLatestObservedTimestamp() + 1;
... instead of adding 1 below?
PS5, Line 1845: For details on
: /// retrieving and using the retrieved server timestamp, see
: /// KuduClient::GetLatestObservedTimestamp().
Can you please change this to something like: "See KuduClient::GetLatestObservedTimestamp() for details on how to use this for Read-Your-Writes."
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 3:
(3 comments)
Thank you for review!
Will post updated version soon.
http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG
Commit Message:
Line 18: David Alves is anticipating some changes there: in short, the timestamp
> oh, also might be better to point to the JIRA (KUDU-611) that to me :)
Thanks for the reference -- I couldn't find that and decided to mention your name instead. Will fix. Sorry for being inconsiderate here.
http://gerrit.cloudera.org:8080/#/c/4569/3/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 386: /// The latest observed timestamp can be used to start a snapshot scan on a
> doesn't have to be in this patch, but it would be good to note that this co
Good idea, will do. Actually, I'll just re-purpose this change list to cover both KUDU-1661 and KUDU-1663.
PS3, Line 1816: one of
> nit: s/one of/a
Done
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4569
to look at the new patch set (#2).
Change subject: [c++ client] added few deprecation notes
......................................................................
[c++ client] added few deprecation notes
KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp()
as experimental
Added deprecation notes for methods related to getting and setting
hybrid timestamps to perform READ_AT_SNAPSHOT scan:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 18 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/2
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 2:
(1 comment)
> (1 comment)
>
> I think that the get/set latest timestamp APIs can be marked with
> "unstable" or "experimental" or something, but I would delay actual
> deprecation until there are replacements.
Thank you for the feedback. I'll try to figure out whether it's possible to deliver that message via method/function attributes.
As for now, I don't know how to do that. The only thing we can do in that regard is note-like doxygen attributes, so we can at least notify users of the API via the documentation.
http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 390: /// @deprecated This method is experimental and will either disappear or
> is there a way to mark it as "unstable" instead of deprecated? seems more a
Unfortunately, there is no way to do that with current version of doxygen.
Instead of @deprecated, we could add one of those notice-like tags:
@note
@warning
@attention
What do you think is the best one in the context?
But of course, we need to remove the deprecated attribute from the method's signature. And I don't know what to put there instead to notify users of the API on the 'unstable' status of this API.
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 1:
(1 comment)
> (1 comment)
>
> Shouldn't we wait until the new API lands before deprecating the
> old one though?
That's a good question.
The original intention for this change was to warn users of the API about the anticipated changes (as David mentioned in HipChat channel).
We could just add a notice, warning or attention (@notice, @warning and @attention in Doxygen lingo) as comments, but I thought if were about to deprecate the methods pretty soon, I could just add the deprecation note along with corresponding clang/gcc attribute.
It would be nice to collect more feedback on this.
http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h
File src/kudu/client/client.h:
PS1, Line 396: its
> Nit: "in signature". Or maybe just "or change in a future release"
Done
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4569
to look at the new patch set (#4).
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
[c++ client] notes for timestamp-related methods
Added notes on anticipated changes on methods related to getting
and setting hybrid timestamps while performing READ_AT_SNAPSHOT
scan operations:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Some changes are anticipated there in the context of KUDU-611.
In short, the timestamp might be replaced with some opaque type
(e.g., a sequence of bytes).
Also, added code sample to provide an idea how to use value returned
by KuduClient::GetLatestObservedTimestamp() to set appropriate snapshot
timestamp via KuduScanner::SetSnapshotRaw().
This addresses the following JIRA issues:
KUDU-1661 Mark KuduClient::GetLatestObservedTimestamp()
as experimental
KUDU-1663 Clean-up in-code documentation for KuduScanner::ReadMode
and KuduClient::GetLatestObservedTimestamp
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 38 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/4
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4569
to look at the new patch set (#5).
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
[c++ client] notes for timestamp-related methods
Added notes on anticipated changes on methods related to getting
and setting hybrid timestamps while performing READ_AT_SNAPSHOT
scan operations:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Some changes are anticipated there in the context of KUDU-611.
In short, the timestamp might be replaced with some opaque type
(e.g., a sequence of bytes).
Also, added code sample to provide an idea how to use value returned
by KuduClient::GetLatestObservedTimestamp() to set appropriate snapshot
timestamp via KuduScanner::SetSnapshotRaw().
This addresses the following JIRA issues:
KUDU-1661 Mark KuduClient::GetLatestObservedTimestamp()
as experimental
KUDU-1663 Clean-up in-code documentation for KuduScanner::ReadMode
and KuduClient::GetLatestObservedTimestamp
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
---
M src/kudu/client/client.h
1 file changed, 39 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/5
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4569/4/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 390: /// Below is an example how to use this method along with
thanks for adding this stuff, you forgot to set the replica selection though.
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [c++ client] added few deprecation notes
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 1:
(1 comment)
Shouldn't we wait until the new API lands before deprecating the old one though?
http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h
File src/kudu/client/client.h:
PS1, Line 396: its
Nit: "in signature". Or maybe just "or change in a future release"
Do the same in the other methods, and in the @deprecated tags.
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
[c++ client] notes for timestamp-related methods
Added notes on anticipated changes on methods related to getting
and setting hybrid timestamps while performing READ_AT_SNAPSHOT
scan operations:
* KuduClient::GetLatestObservedTimestamp()
* KuduClient::SetLatestObservedTimestamp()
* KuduScanner::SetSnapshotRaw()
Some changes are anticipated there in the context of KUDU-611.
In short, the timestamp might be replaced with some opaque type
(e.g., a sequence of bytes).
Also, added code sample to provide an idea how to use value returned
by KuduClient::GetLatestObservedTimestamp() to set appropriate snapshot
timestamp via KuduScanner::SetSnapshotRaw().
This addresses the following JIRA issues:
KUDU-1661 Mark KuduClient::GetLatestObservedTimestamp()
as experimental
KUDU-1663 Clean-up in-code documentation for KuduScanner::ReadMode
and KuduClient::GetLatestObservedTimestamp
Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Reviewed-on: http://gerrit.cloudera.org:8080/4569
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/client/client.h
1 file changed, 43 insertions(+), 1 deletion(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4569/4/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 390: /// Below is an example how to use this method along with
> thanks for adding this stuff, you forgot to set the replica selection thoug
My bad. It's good you spotted the missing part -- it seems I copied that part of the code from JIRA issue description (will update it there as well).
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 390: /// @deprecated This method is experimental and will either disappear or
> Unfortunately, there is no way to do that with current version of doxygen.
I'd go for adding a @note and dropping the @deprecated tag, or just add "Note: This API is unstable." without any tags at all if you don't think there are any that make sense.
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 2:
(1 comment)
I think that the get/set latest timestamp APIs can be marked with "unstable" or "experimental" or something, but I would delay actual deprecation until there are replacements.
http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 390: /// @deprecated This method is experimental and will either disappear or
is there a way to mark it as "unstable" instead of deprecated? seems more accurate
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [c++ client] added few deprecation notes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG
Commit Message:
Line 18: David Alves is anticipating some changes there: in short, the timestamp
> Thanks for the reference -- I couldn't find that and decided to mention you
not being inconsiderate, just that JIRA is more persistent than my memory :)
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] notes for timestamp-related methods
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] notes for timestamp-related methods
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4569/5/src/kudu/client/client.h
File src/kudu/client/client.h:
PS5, Line 388: See
> lost word.
Done
Line 390: /// Below is an example how to use this method along with
> Can you preface this by:
Done
Line 405: /// uint64_t snapshot_timestamp = client->GetLatestObservedTimestamp();
> can you change this to:
Done
PS5, Line 1845: For details on
: /// retrieving and using the retrieved server timestamp, see
: /// KuduClient::GetLatestObservedTimestamp().
> Can you please change this to something like: "See KuduClient::GetLatestObs
I added that as a separate paragraph for this method: I think it's even better.
--
To view, visit http://gerrit.cloudera.org:8080/4569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes