You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org> on 2016/09/21 01:27:02 UTC
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Jean-Daniel Cryans has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4487
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
[java client] Improve and hide OperationResponse#getWriteTimestamp
That method was returning a HT-encoded ts, not a ts in microseconds. It's
also not meant for public consumption just yet, the same way
AbstractKuduScannerBuilder#snapshotTimestampRaw is.
Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
---
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
2 files changed, 9 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/4487/1
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
> If you allow me I'll do this in a separate patch that also sets up the rele
Sure.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
> Oh, I misunderstood.
If you allow me I'll do this in a separate patch that also sets up the release notes for the next version.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
> My previous comment was an attempt at convincing you that we don't need to
Oh, I misunderstood.
I think it should be documented anyway. It's cheap, it's a good habit to get into, and it leaves a paper trail for us later.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
Line 81: * @return a long representing a HybridClock-encoded timestamp,
we call it hybridtime elsewhere. also iirc we actually always return the write timestamp.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
Should probably add this to the release notes for 1.1.0.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4487
to look at the new patch set (#2).
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
[java client] Improve and hide OperationResponse#getWriteTimestamp
That method was returning a HT-encoded ts, not a ts in microseconds. It's
also not meant for public consumption just yet, the same way
AbstractKuduScannerBuilder#snapshotTimestampRaw is.
Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
3 files changed, 8 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/4487/2
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
[java client] Improve and hide OperationResponse#getWriteTimestamp
That method was returning a HT-encoded ts, not a ts in microseconds. It's
also not meant for public consumption just yet, the same way
AbstractKuduScannerBuilder#snapshotTimestampRaw is.
Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Reviewed-on: http://gerrit.cloudera.org:8080/4487
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
3 files changed, 8 insertions(+), 8 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
> Yeah, although I doubt anybody is using this. I can also change the method'
So what's the verdict? To doc or not? I don't see any release notes change in PS2.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
PS1, Line 85: @InterfaceAudience.Private
> So what's the verdict? To doc or not? I don't see any release notes change
My previous comment was an attempt at convincing you that we don't need to doc it.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4487/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java:
Line 81: * @return a long representing a HybridClock-encoded timestamp,
> we call it hybridtime elsewhere. also iirc we actually always return the wr
I'll change the HybridClock in AbstractKuduScannerBuilder too then.
And it looks like you're right, the ts is always there.
PS1, Line 85: @InterfaceAudience.Private
> Should probably add this to the release notes for 1.1.0.
Yeah, although I doubt anybody is using this. I can also change the method's name in a backward compatible way.
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Improve and hide OperationResponse#getWriteTimestamp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java client] Improve and hide OperationResponse#getWriteTimestamp
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cfc6fcc1d0607a94bb8be9e5a0d53a4987920af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No