[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/4781/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: PS2, Line 975: relatedRpc > nit: does it makes sense to rename the parameter as well: relatedRpc --> pa Yup, already did that for the next revision. http://gerrit.cloudera.org:8080/#/c/4781/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java: PS2, Line 238: this.parentRpc = parentRpc > Nit: does it make sense to protect against setting parentRpc to itself? Lemme add an assert. http://gerrit.cloudera.org:8080/#/c/4781/2/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java: PS2, Line 568: return an exception if we couldn't dispatch the error, or null > nit: probably, this out of the scope, but just in case: do we use @return t This is just Javadoc standards, here I use @return because it's what this method returns. You'd use @throws if it was thrown. -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1715. Add a way to set ReplicaSelection to the java client
Jean-Daniel Cryans has uploaded a new patch set (#3). Change subject: KUDU-1715. Add a way to set ReplicaSelection to the java client .. KUDU-1715. Add a way to set ReplicaSelection to the java client This patch adds a ReplicaSelection option for all the RPCs, which always defaults to LEADER_ONLY, as well as a way to change it for scanners. Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/ReplicaSelection.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java 9 files changed, 115 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/4837/3 -- To view, visit http://gerrit.cloudera.org:8080/4837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 2: This is just a rebase on top of my other Java patches that I'm now depending on in this patch. -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java client] Refactor all server info into a single class, add locality
Jean-Daniel Cryans has uploaded a new patch set (#3). Change subject: [java client] Refactor all server info into a single class, add locality .. [java client] Refactor all server info into a single class, add locality Having this helps propagate information around the client. RemoteTablet can now answer requests for local servers. This patch also tries to simplify how RemoteTablets are created, as it's getting a bit messy and redundant with the passing of ServerInfos. Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java 7 files changed, 177 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4836/3 -- To view, visit http://gerrit.cloudera.org:8080/4836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has uploaded a new patch set (#2).
Change subject: WIP [java client] Implement RPC tracing
..
WIP [java client] Implement RPC tracing
First stab at getting traces in the Java client. This patch is using a pretty
simple
method if just shoving container objects into a list, per RPC. The traces are
lightweight
and don't try anything fancy. We also introduce the concept of "parent RPC", so
that say
a Write RPC might spawn a GetTableLocations, and the latter will be added to
the former
so that the call to the master adds traces to both RPCs.
This patch isn't adding a nice way to present the traces (like JSON) but here's
a simple
toString example:
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973546,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973547,
action=SEND_TO_SERVER, server=3926a6a73e994152be1336beb434154e},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973548,
action=RECEIVE_FROM_SERVER, server=3926a6a73e994152be1336beb434154e,
callStatus=Network error: [Peer 3926a6a73e994152be1336beb434154e] Connection
reset on [id: 0xc83743df]}
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973548,
action=SLEEP_THEN_RETRY, callStatus=Network error: [Peer
3926a6a73e994152be1336beb434154e] Connection reset on [id: 0xc83743df]},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973574,
action=QUERY_MASTER},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973574,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973574,
action=SEND_TO_SERVER, server=c0d4588690d241c69821ee773eebd185},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973576,
action=RECEIVE_FROM_SERVER, server=c0d4588690d241c69821ee773eebd185,
callStatus=OK},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973579,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973579,
action=SEND_TO_SERVER, server=0353a6d97d6c49f9a727bc1ee6c3393e},
Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeleteTableRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetMasterRegistrationRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/IsAlterTableDoneRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ListTablesRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/ListTabletServersRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ListTabletsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
19 files changed, 229 insertions(+), 49 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/4781/2
--
To view, visit http://gerrit.cloudera.org:8080/4781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1715. Add a way to set ReplicaSelection to the java client
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-1715. Add a way to set ReplicaSelection to the java client
..
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4837/2/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java:
PS2, Line 319: us
> Nit: use
Done
http://gerrit.cloudera.org:8080/#/c/4837/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java:
PS2, Line 265: ReplicaSelection getReplicaSelection() {
: return ReplicaSelection.LEADER_ONLY;
: }
> why is this hard-coded to "leader only"?
We basically make it impossible to configure, unless the actual RPCs (like
scanners) allow it.
http://gerrit.cloudera.org:8080/#/c/4837/2/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
PS2, Line 170: us
> nit: use
Done
PS2, Line 171: a UUID for the server that matches the selection, can be null
> how can this be null? I think we should put some thought into whether we tr
It is a hint, it works the way you expect it. So it can be null if we don't
even know where the leader might be, for example.
Line 180: return null;
> Uh, shouldn't this just throw something?
Can throw an unchecked exception yeah.
http://gerrit.cloudera.org:8080/#/c/4837/2/java/kudu-client/src/main/java/org/apache/kudu/client/ReplicaSelection.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/ReplicaSelection.java:
PS2, Line 33: random one
> is this actually true? are the replicas really selected at random? or does
This is lifted straight out of client.h, and as far as the user can tell it
will be random (non-deterministic would be a better word given the
implementation in the Java client).
http://gerrit.cloudera.org:8080/#/c/4837/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java:
Line 180: public void testLocalScanner() throws Exception {
> I'm assuming that you omitted the "leader only" case because it's the defau
Sure thing.
--
To view, visit http://gerrit.cloudera.org:8080/4837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-HasComments: Yes
[kudu-CR] [java client] Refactor all server info into a single class, add locality
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Refactor all server info into a single class, add locality .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4836/2/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java: PS2, Line 38: once it is init()'d > please update Done PS2, Line 151: or null if this :* cache doesn't know of any servers > Is this actually possible? Wouldn't we reject the creation of a RemoteTable Super stale cache and all the replicas moved and we, for some reason, don't query the master each time we find a tablet has moved. Almost impossible. Line 163: return lastUuid; > http://dilbert.com/strip/2001-10-25 Hey that's how this patch failed the first time, values() returned things in a different order on my machine and on Linux. http://gerrit.cloudera.org:8080/#/c/4836/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: Line 26: @InterfaceStability.Evolving > Is this needed for Private interfaces? Not really, I'll remove. -- To view, visit http://gerrit.cloudera.org:8080/4836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: Yes
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4781/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java: Line 233:* only one related RPC. > worth documenting that you should be careful not to cause any cycle in "rel Good idea, I wasn't super happy with "related" but when we talked about a while ago that's the name we came up with. "parent" in this implementation makes more sense. http://gerrit.cloudera.org:8080/#/c/4781/1/java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java: PS1, Line 33: private final TabletClient server > Is it possible to store just the identifier here? My concern is that a tra I think we'll want to print more information, but it just so happens that I have this other patch https://gerrit.cloudera.org/#/c/4836/ that adds a new class that we could use here instead :) http://gerrit.cloudera.org:8080/#/c/4781/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java: PS1, Line 225: sendRpc(rpc); > Is it necessary to add a trace here as well? No, you'd get double-logging since you'd add it here then again on line 162. -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1715. Add a way to set ReplicaSelection to the java client
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1715. Add a way to set ReplicaSelection to the java client .. Patch Set 2: > Removed the following votes: > > * Verified-1 by Kudu Jenkins (120) All Java tests are passing, dist test is not working. -- To view, visit http://gerrit.cloudera.org:8080/4837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] [java client] Refactor all server info into a single class, add locality
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Refactor all server info into a single class, add locality .. Patch Set 2: > Removed the following votes: > > * Verified-1 by Kudu Jenkins (120) All Java tests are passing, dist test is not working. -- To view, visit http://gerrit.cloudera.org:8080/4836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] Add krb5 dependency to Java README
Jean-Daniel Cryans has posted comments on this change. Change subject: Add krb5 dependency to Java README .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5e4ffe0bee242ddec3a9effd4c3404348f1b889c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Refactor all server info into a single class, add locality
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4836 to look at the new patch set (#2). Change subject: [java client] Refactor all server info into a single class, add locality .. [java client] Refactor all server info into a single class, add locality Having this helps propagate information around the client. RemoteTablet can now answer requests for local servers. This patch also tries to simplify how RemoteTablets are created, as it's getting a bit messy and redundant with the passing of ServerInfos. Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java 7 files changed, 178 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4836/2 -- To view, visit http://gerrit.cloudera.org:8080/4836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1715. Add a way to set ReplicaSelection to the java client
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4837 Change subject: KUDU-1715. Add a way to set ReplicaSelection to the java client .. KUDU-1715. Add a way to set ReplicaSelection to the java client This patch adds a ReplicaSelection option for all the RPCs, which always defaults to LEADER_ONLY, as well as a way to change it for scanners. Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/ReplicaSelection.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java 9 files changed, 109 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/4837/1 -- To view, visit http://gerrit.cloudera.org:8080/4837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3bb08c9c78271a0065c8aa8fb9b0f3301f84e828 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Refactor all server info into a single class, add locality
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4836 Change subject: [java client] Refactor all server info into a single class, add locality .. [java client] Refactor all server info into a single class, add locality Having this helps propagate information around the client. RemoteTablet can now answer requests for local servers. This patch also tries to simplify how RemoteTablets are created, as it's getting a bit messy and redundant with the passing of ServerInfos. Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java 7 files changed, 178 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4836/1 -- To view, visit http://gerrit.cloudera.org:8080/4836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I33984a437d8c8d07d5db4d16f8da723b3e904189 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Identify client-local TabletClients
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Identify client-local TabletClients .. [java client] Identify client-local TabletClients This patch adds building blocks to enable selecting the closest replica. Another patch will be required to finish the plumbing job. Inspired by Hadoop: https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java#L698 Change-Id: Ia1bfcbc6b6aea7cb9610e8ae934bf08ab0774ee3 Reviewed-on: http://gerrit.cloudera.org:8080/4786 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java 6 files changed, 97 insertions(+), 43 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia1bfcbc6b6aea7cb9610e8ae934bf08ab0774ee3 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] MiniKdc for Java
Jean-Daniel Cryans has posted comments on this change.
Change subject: MiniKdc for Java
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4788/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java:
Line 1: package org.apache.kudu.client;
Missing license.
PS1, Line 28: An
nit: A
Line 30: */
Maybe explain how this class works? Like it uses external processes for about
everything, how it does it, etc.
PS1, Line 132: potentially race
nit: race-prone?
PS1, Line 178: C
nit: c
Line 322: private static String getBinaryPath(String executable, List
searchPaths) throws IOException {
nit: long line
http://gerrit.cloudera.org:8080/#/c/4788/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKdc.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKdc.java:
Line 1: package org.apache.kudu.client;
Missing license.
--
To view, visit http://gerrit.cloudera.org:8080/4788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24eaa94fae14ca91fb4fdd2deae1f9aec58438b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [java client] Identify client-local TabletClients
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4786 Change subject: [java client] Identify client-local TabletClients .. [java client] Identify client-local TabletClients This patch adds building blocks to enable selecting the closest replica. Another patch will be required to finish the plumbing job. Inspired by Hadoop: https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java#L698 Change-Id: Ia1bfcbc6b6aea7cb9610e8ae934bf08ab0774ee3 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java 6 files changed, 97 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/4786/1 -- To view, visit http://gerrit.cloudera.org:8080/4786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia1bfcbc6b6aea7cb9610e8ae934bf08ab0774ee3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 1: > also the patch should have at least some kind of test I think I didn't want to get too deep, this is a WIP :) -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 1: > haven't looked at the code yet, but a couple quick thoughts: > - let's make sure the trace buffer is bounded, so that if we have > some sort of tight loop retry bug we don't just make the user app > OOME Will do, right now it's unlimited. > - probably worth doing a sanity check of perf using YCSB or > something to make sure the tracing isn't costing too much (I think > a few percent is fine, but if it's a lot we should look at > optimizations before committing) Planning on doing that! -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP [java client] Implement RPC tracing .. Patch Set 1: > Removed the following votes: > > * Verified-1 by Kudu Jenkins (120) Unrelated failure. -- To view, visit http://gerrit.cloudera.org:8080/4781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP [java client] Implement RPC tracing
Jean-Daniel Cryans has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4781
Change subject: WIP [java client] Implement RPC tracing
..
WIP [java client] Implement RPC tracing
First stab at getting traces in the Java client. This patch is using a pretty
simple
method if just shoving container objects into a list, per RPC. The traces are
lightweight
and don't try anything fancy. We also introduce the concept of "related RPC",
so that say
a Write RPC might spawn a GetTableLocations, and the latter will be added to
the former
so that the call to the master adds traces to both RPCs.
This patch isn't adding a nice way to present the traces (like JSON) but here's
a simple
toString example:
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973546,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973547,
action=SEND_TO_SERVER, server=3926a6a73e994152be1336beb434154e},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973548,
action=RECEIVE_FROM_SERVER, server=3926a6a73e994152be1336beb434154e,
callStatus=Network error: [Peer 3926a6a73e994152be1336beb434154e] Connection
reset on [id: 0xc83743df]}
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973548,
action=SLEEP_THEN_RETRY, callStatus=Network error: [Peer
3926a6a73e994152be1336beb434154e] Connection reset on [id: 0xc83743df]},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973574,
action=QUERY_MASTER},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973574,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973574,
action=SEND_TO_SERVER, server=c0d4588690d241c69821ee773eebd185},
RpcTraceObject{rpcMethod='GetTableLocations', timestampMs=1477079973576,
action=RECEIVE_FROM_SERVER, server=c0d4588690d241c69821ee773eebd185,
callStatus=OK},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973579,
action=PICKED_REPLICA},
RpcTraceObject{rpcMethod='Write', timestampMs=1477079973579,
action=SEND_TO_SERVER, server=0353a6d97d6c49f9a727bc1ee6c3393e},
Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeleteTableRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetMasterRegistrationRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/IsAlterTableDoneRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ListTablesRequest.java
M
java/kudu-client/src/main/java/org/apache/kudu/client/ListTabletServersRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ListTabletsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
19 files changed, 228 insertions(+), 49 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/4781/1
--
To view, visit http://gerrit.cloudera.org:8080/4781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR](gh-pages) Add 10/20 weekly update
Jean-Daniel Cryans has posted comments on this change. Change subject: Add 10/20 weekly update .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/4775/1/_posts/2016-10-20-weekly-update.md File _posts/2016-10-20-weekly-update.md: Line 42: I resent the lack of link to rack awareness. -- To view, visit http://gerrit.cloudera.org:8080/4775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98bbe5bd3d16a90a99dacd793e0bf2e2fea1b50d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: Yes
[kudu-CR] [java client] Decouple TabletClient from RemoteTablet
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4757 to look at the new patch set (#2). Change subject: [java client] Decouple TabletClient from RemoteTablet .. [java client] Decouple TabletClient from RemoteTablet RemoteTablet was caching TabletClients which was making it hard to test it in isolation, since it required having real connections to servers. This patch makes it so that only UUIDs are passed around, the client now queries a RemoteTablet to know which UUID to get from ConnectionCache. A lot of new unit tests were written that weren't possible before without a lot of mocking. Doing this brings subtle behavior changes. ConnectionCache is now the only component responsible for handling TabletClients. The whole concept of "reconnection" could be brought into ConnectionCache, simplifying sendRpcToTablet. It also means that using an object monitor likely creates a bottleneck in ConnectionCache, so a RWL was deployed instead. Since I was in that code, I also changed the tablet ID from Slice to String in RemoteTablet. It was an old mistake I made years ago, we didn't need the Slice. This still needs to be benchmarked against the 1.0.x client jar. Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java A java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java A java/kudu-client/src/main/java/org/apache/kudu/client/PingResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java 17 files changed, 628 insertions(+), 525 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/4757/2 -- To view, visit http://gerrit.cloudera.org:8080/4757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Decouple TabletClient from RemoteTablet
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Decouple TabletClient from RemoteTablet
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java:
PS1, Line 49: }
> {@link TabletClient}s are currently...
Done
Line 95: void connectTS(Master.TSInfoPB tsInfoPB) throws UnknownHostException
{
> private or @VisibleForTesting (here and elsewhere)
This is used by AsyncKuduClient.
PS1, Line 99: {uuid}
> This doesn't work correctly, the braces have to be empty.
Done
Line 167: readLock.lock();
> call getClient here instead of dealing with locks.
Done
Line 187: ArrayList> deferreds = new ArrayList<>();
> I think it's still an improvement to move the ArrayList creation inside the
Ok I'll just move everything in then.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
Line 70: synchronized (tabletServers) {
> I think it's fine in the ctor. There's some kind of happens before relatio
Done
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:
Line 623:* This method is not synchronized. You need to synchronize on this
> Should this be updated?
Oh right, thanks!
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java:
Line 298: static Common.PartitionPB.Builder getFakePartitionPB() {
> This is a valid partition. Not sure if that matters or not.
I won't say bogus then, but it is fake as it's not represented on a cluster.
--
To view, visit http://gerrit.cloudera.org:8080/4757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Decouple TabletClient from RemoteTablet
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Decouple TabletClient from RemoteTablet .. Patch Set 1: Additionally, I'm just done comparing runs of YCSB load with the 1.0.1 client jar and master patched with this change. The perf was within the same ballpark (lots of variance), always a bit faster but I wouldn't say it's because of this change. I also inspected jstacks which all looked ok. -- To view, visit http://gerrit.cloudera.org:8080/4757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Decouple TabletClient from RemoteTablet
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Decouple TabletClient from RemoteTablet
..
Patch Set 1:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
PS1, Line 1287: if (!lookupExceptions.isEmpty() &&
: lookupExceptions.size() ==
tabletPb.getReplicasCount()) {
: Status statusIOE = Status.IOError("Couldn't find any
valid locations, exceptions: " +
: lookupExceptions);
: throw new NonRecoverableException(statusIOE);
: }
> Shouldn't this be outside the inner loop?
I guess it'd be cleaner, but as far as I can tell it does the same thing.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java:
PS1, Line 77: private final Lock readLock = lock.readLock();
: private final Lock writeLock = lock.readLock();
> This is just for shorthand, right?
Yup!
Line 127: uuid2client.put(uuid, client);
> So if we reconnect to a server we thought was "dead", we'll evict it from u
Yes, should I add a comment?
Line 187: ArrayList> deferreds = new ArrayList<>();
> Nit: could preallocate this list with uuid2client.size().
That'd require locking twice... didn't think it was a big deal since this
happens only when shutting down the client.
PS1, Line 200: Modifying the list returned by this method won't affect this
cache
> Maybe return a Guava ImmutableList (either directly, or hidden behind a Lis
Done
Line 261: private final class TabletClientPipeline extends
DefaultChannelPipeline {
> What was the effect of removing all the code here?
We used to have to lookup the hostnames, but it wasn't necessary since a while
ago I added them in TabletClient.
Then it doesn't really matter if we remove all the TabletClients or not, as
long as we shut them down, which is why I added a unit test that verifies that
we do it.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java:
Line 27: *
> ?
Fill it with your imagination.
I added something at some point, then ctrl-z'd a bunch, forgot I had to add
that back.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
Line 56: private final ArrayList tabletServers = new ArrayList<>();
> Would it be simpler to make this a Set? Certainly add/remove will get simpl
Indeed, that's even better.
Line 70: synchronized (tabletServers) {
> Shouldn't be necessary in the constructor; no other thread could have a ref
Won't GuardedBy complain?
PS1, Line 71: tabletServers.clear();
: leaderIndex = NO_LEADER_INDEX;
> Also unnecessary; these are their initial values.
Forgot to remove them, thanks.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java:
Line 109: allDead = true;
> I'm confused by this; why does the fact that there was at least one tablet
err allDead should be next to break below. We're failing if we don't have
servers because we're expecting to have connections to disconnect.
http://gerrit.cloudera.org:8080/#/c/4757/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java:
Line 27: public class TestConnectionCache {
> Why not use BaseKuduTest or whatever for this test?
To be able to use the mini cluster directly.
PS1, Line 54: waitForConnectionToDie
> Somewhat odd that join() on shutdown() doesn't automatically take care of t
Yeah I spent some time trying to figure that one out... something something
shenanigans.
PS1, Line 77: 5000
> Not DEFAULT_SLEEP?
Right without BaseKuduTest that's not reachable.
--
To view, visit http://gerrit.cloudera.org:8080/4757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Decouple TabletClient from RemoteTablet
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4757 Change subject: [java client] Decouple TabletClient from RemoteTablet .. [java client] Decouple TabletClient from RemoteTablet RemoteTablet was caching TabletClients which was making it hard to test it in isolation, since it required having real connections to servers. This patch makes it so that only UUIDs are passed around, the client now queries a RemoteTablet to know which UUID to get from ConnectionCache. A lot of new unit tests were written that weren't possible before without a lot of mocking. Doing this brings subtle behavior changes. ConnectionCache is now the only component responsible for handling TabletClients. The whole concept of "reconnection" could be brought into ConnectionCache, simplifying sendRpcToTablet. It also means that using an object monitor likely creates a bottleneck in ConnectionCache, so a RWL was deployed instead. Since I was in that code, I also changed the tablet ID from Slice to String in RemoteTablet. It was an old mistake I made years ago, we didn't need the Slice. This still needs to be benchmarked against the 1.0.x client jar. Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java A java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java A java/kudu-client/src/main/java/org/apache/kudu/client/PingResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java 17 files changed, 612 insertions(+), 469 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/4757/1 -- To view, visit http://gerrit.cloudera.org:8080/4757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If3ad2190c7e2c7f51cb9ffe6ed3348b62488e675 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Extract RemoteTablet from AsyncKuduClient .. [java client] Extract RemoteTablet from AsyncKuduClient RemoteTablet is responsible for handling the Java client's view of where replicas are for its tablet, and who the leader is. Extracting this bit of functionality is important if we want to be able to unit test it without bringing up a whole client and possibly a cluster. This patch makes a minor attempt at simplifying the interfaces, with more work to come. Likewise for unit tests. Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Reviewed-on: http://gerrit.cloudera.org:8080/4719 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 8 files changed, 345 insertions(+), 318 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [client.h] CountBufferedOperations marked as deprecated
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [client.h] CountBufferedOperations marked as deprecated .. [client.h] CountBufferedOperations marked as deprecated KuduSession::CountBufferedOperations() is used only by tests now. Also, the only flush mode to use this method consistently is MANUAL_FLUSH mode, but in that mode it's easy to count for number of buffered operations by a different means -- that's the count of Apply() calls since last KuduSession::Flush() call (or invocation of the callback passed into KuduSession::FlushAsync() method). Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548 Reviewed-on: http://gerrit.cloudera.org:8080/4723 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans --- M src/kudu/client/client.h 1 file changed, 17 insertions(+), 5 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [client.h] CountBufferedOperations marked as deprecated
Jean-Daniel Cryans has posted comments on this change. Change subject: [client.h] CountBufferedOperations marked as deprecated .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] ITBLL: use a faster PRNG
Jean-Daniel Cryans has posted comments on this change. Change subject: ITBLL: use a faster PRNG .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f51664af25b9fb4309dd78556e954bf483d22c0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Extract RemoteTablet from AsyncKuduClient
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4719/3/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
Line 55: * Unlike the C++ client, we don't short-circuit the call to the
master if it isn't available.
As we discussed, I'm also removing this.
Line 76: private int leaderIndex = NO_LEADER_INDEX;
> Add @GuardedBy("tabletServers") to this.
Done
--
To view, visit http://gerrit.cloudera.org:8080/4719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4719 to look at the new patch set (#4). Change subject: [java client] Extract RemoteTablet from AsyncKuduClient .. [java client] Extract RemoteTablet from AsyncKuduClient RemoteTablet is responsible for handling the Java client's view of where replicas are for its tablet, and who the leader is. Extracting this bit of functionality is important if we want to be able to unit test it without bringing up a whole client and possibly a cluster. This patch makes a minor attempt at simplifying the interfaces, with more work to come. Likewise for unit tests. Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 8 files changed, 345 insertions(+), 318 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4719/4 -- To view, visit http://gerrit.cloudera.org:8080/4719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Extract ip2client out of AsyncKuduClient .. [java client] Extract ip2client out of AsyncKuduClient As part of making the Java client more modular and easier to test, this patch is moving almost all of the connections management into a separate class. The change was rather painless. Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Reviewed-on: http://gerrit.cloudera.org:8080/4717 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 3 files changed, 417 insertions(+), 355 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [client.h] updated comment for CountBufferedOperations
Jean-Daniel Cryans has posted comments on this change. Change subject: [client.h] updated comment for CountBufferedOperations .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4723/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 1493: This is different than HasPendingOperations() above > Yep -- so far I can see, this method is used only in tests. However, MJ fo Well that MJ was interested is... interesting since Impala will be using AUTO_FLUSH_BACKGROUND, which this method is _not_ useful for, right? To handle the change, I'd duplicate and mark this one deprecated. -- To view, visit http://gerrit.cloudera.org:8080/4723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[kudu-CR] [client.h] updated comment for CountBufferedOperations
Jean-Daniel Cryans has posted comments on this change. Change subject: [client.h] updated comment for CountBufferedOperations .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4723/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 1488: nit: extra blank space. PS1, Line 1493: This is different than HasPendingOperations() above It makes you wonder if we want this method at all. On the Java side there's no equivalent. Maybe a better name would be "CountOperationsToFlush"? -- To view, visit http://gerrit.cloudera.org:8080/4723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4717 to look at the new patch set (#3). Change subject: [java client] Extract ip2client out of AsyncKuduClient .. [java client] Extract ip2client out of AsyncKuduClient As part of making the Java client more modular and easier to test, this patch is moving almost all of the connections management into a separate class. The change was rather painless. Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 3 files changed, 417 insertions(+), 355 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4717/3 -- To view, visit http://gerrit.cloudera.org:8080/4717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4717 to look at the new patch set (#2). Change subject: [java client] Extract ip2client out of AsyncKuduClient .. [java client] Extract ip2client out of AsyncKuduClient As part of making the Java client more modular and easier to test, this patch is moving almost all of the connections management into a separate class. The change was rather painless. Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 3 files changed, 417 insertions(+), 355 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4717/2 -- To view, visit http://gerrit.cloudera.org:8080/4717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4719 to look at the new patch set (#2). Change subject: [java client] Extract RemoteTablet from AsyncKuduClient .. [java client] Extract RemoteTablet from AsyncKuduClient RemoteTablet is responsible for handling the Java client's view of where replicas are for its tablet, and who the leader is. Extracting this bit of functionality is important if we want to be able to unit test it without bringing up a whole client and possibly a cluster. This patch makes a minor attempt at simplifying the interfaces, with more work to come. Likewise for unit tests. Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 8 files changed, 348 insertions(+), 318 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4719/2 -- To view, visit http://gerrit.cloudera.org:8080/4719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Extract RemoteTablet from AsyncKuduClient .. Patch Set 1: (4 comments) > (4 comments) > > As with the other patch, has anything actually changed (besides > method visibility) in the moved code? As you saw I moved the getting of a TabletClient directly into RemoteTablet, and that's about it. http://gerrit.cloudera.org:8080/#/c/4719/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: PS1, Line 589: tablet == null > I presume that in the transition from clientFor() to tablet.getLeaderConnec Yes. So the tablet can be set to null to "invalidate" the scan, but looking through it I don't think it's possible to get here with a null tablet? I'll put an assert. PS1, Line 689: getConnectionToLeader() > This method doesn't exist. Oops that got lost in the refactorings. Line 1304: rt.init(tabletPb); > Can we do this as part of createTabletFromPb()? In fact, maybe createTablet Can't do it in the ctor, it throws an exception. I like the static RemoteTablet idea. http://gerrit.cloudera.org:8080/#/c/4719/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java: Line 74: private final ConnectionCache connectionCache; > Based on our discussion, sounds like this will go away because RemoteTablet Right, this could go away at some point. -- To view, visit http://gerrit.cloudera.org:8080/4719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Extract ip2client out of AsyncKuduClient
..
Patch Set 1:
(3 comments)
> (3 comments)
>
> I looked at the ConnectionCache API and members, but only skimmed
> the method implementations because I assumed that was just code
> motion. Was there any implementation change worth looking at?
No.
http://gerrit.cloudera.org:8080/#/c/4717/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java:
Line 91: this.kuduClient = client;
> Isn't this a reference cycle between the AsyncKuduClient and ConnectionCach
Java is more advanced than you think ;)
Also the client itself is needed in TabletClientPipeline.
PS1, Line 167: AsyncKuduClient
> Odd to reference this class here, no?
Done
Line 279: static String getIP(final String host) {
> Since this is static, I think it could remain in AsyncKuduClient, to keep t
It's used in both classes, so I'd rather keep the connection-related stuff in
this class. AsyncKuduClient is already big enough.
--
To view, visit http://gerrit.cloudera.org:8080/4717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. [java client] Cleanup AsyncKuduClient's unused caches Originally, asynchbase came with a few caches such as server-connection->region and region->server-connection. Via dozens of patches, we've reduced their usefulness to 0. This patch simple finishes the job and removes them. FWIW client2tablets was always a source of pain, and the caching logic is now a lot easier to manage, and potentially refactor! Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Reviewed-on: http://gerrit.cloudera.org:8080/4705 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 2 files changed, 22 insertions(+), 80 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4719 Change subject: [java client] Extract RemoteTablet from AsyncKuduClient .. [java client] Extract RemoteTablet from AsyncKuduClient RemoteTablet is responsible for handling the Java client's view of where replicas are for its tablet, and who the leader is. Extracting this bit of functionality is important if we want to be able to unit test it without bringing up a whole client and possibly a cluster. This patch makes a minor attempt at simplifying the interfaces, with more work to come. Likewise for unit tests. Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 8 files changed, 337 insertions(+), 309 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4719/1 -- To view, visit http://gerrit.cloudera.org:8080/4719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. Patch Set 4: > So no regression test for that race? Sorry, where in this gerrit did you mention a regression test? :P TBH right now it'd be a pain. Right now I'm doing even more refactoring, I think testing will soon become a lot easier. -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4717 Change subject: [java client] Extract ip2client out of AsyncKuduClient .. [java client] Extract ip2client out of AsyncKuduClient As part of making the Java client more modular and easier to test, this patch is moving almost all of the connections management into a separate class. The change was rather painless. Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 3 files changed, 417 insertions(+), 355 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4717/1 -- To view, visit http://gerrit.cloudera.org:8080/4717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/4705/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: PS2, Line 144: /** :* This map contains data cached from calls to the master's :* GetTableLocations RPC. This map is keyed by table ID. :*/ : private final ConcurrentHashMap tableLocations = : > Nit: just combine the two paragraphs, like "This map contains data cached f Done Line 178: > Could you add a "GuardedBy" annotation to this? Done Line 1360: Slice tabletId = rt.tabletId; > This comment is probably unnecessary now. Done Line 2008: String ip = getIP(host); > If this has to be called with tabletServers synchronized, then it should be Done -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1682. Lock contention on table locations cache in Java client
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-1682. Lock contention on table locations cache in Java client .. KUDU-1682. Lock contention on table locations cache in Java client Change-Id: I0f6ba8f4fced6f043f7132fd11078044b004ea21 Reviewed-on: http://gerrit.cloudera.org:8080/4706 Reviewed-by: Adar Dembo Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java 1 file changed, 23 insertions(+), 6 deletions(-) Approvals: Dan Burkert: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4706 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0f6ba8f4fced6f043f7132fd11078044b004ea21 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4705 to look at the new patch set (#3). Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. [java client] Cleanup AsyncKuduClient's unused caches Originally, asynchbase came with a few caches such as server-connection->region and region->server-connection. Via dozens of patches, we've reduced their usefulness to 0. This patch simple finishes the job and removes them. FWIW client2tablets was always a source of pain, and the caching logic is now a lot easier to manage, and potentially refactor! Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 2 files changed, 22 insertions(+), 80 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4705/3 -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1682. Lock contention on table locations cache in Java client
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4706 Change subject: KUDU-1682. Lock contention on table locations cache in Java client .. KUDU-1682. Lock contention on table locations cache in Java client Change-Id: I0f6ba8f4fced6f043f7132fd11078044b004ea21 --- M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java 1 file changed, 23 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/4706/1 -- To view, visit http://gerrit.cloudera.org:8080/4706 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0f6ba8f4fced6f043f7132fd11078044b004ea21 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Jean-Daniel Cryans has uploaded a new patch set (#2). Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. [java client] Cleanup AsyncKuduClient's unused caches Originally, asynchbase came with a few caches such as server-connection->region and region->server-connection. Via dozens of patches, we've reduced their usefulness to 0. This patch simple finishes the job and removes them. FWIW client2tablets was always a source of pain, and the caching logic is now a lot easier to manage, and potentially refactor! Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 2 files changed, 18 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4705/2 -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Cleanup AsyncKuduClient's unused caches
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4705 Change subject: [java client] Cleanup AsyncKuduClient's unused caches .. [java client] Cleanup AsyncKuduClient's unused caches Originally, asynchbase came with a few caches such as server-connection->region and region->server-connection. Via dozens of patches, we've reduced their usefulness to 0. This patch simple finishes the job and removes them. FWIW client2tablets was always a source of pain, and the caching logic is now a lot easier to manage, and potentially refactor! Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 2 files changed, 10 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4705/1 -- To view, visit http://gerrit.cloudera.org:8080/4705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I62802c34c618c83a4ff69d79825387cbe4ab51a8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] add 1.0.1 release notes to prior release notes.adoc
Jean-Daniel Cryans has posted comments on this change. Change subject: add 1.0.1 release notes to prior_release_notes.adoc .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieff57fd7120e98e2e1a7c0812e6c04eca704c6ad Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] AsyncKuduClient#delayedSendRpcToTablet should
return a Deferred
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4654/1//COMMIT_MSG
Commit Message:
PS1, Line 12: paths that simply handle background retries (don't
: need a Deferred since we rely on KuduRpc#callback).
> Can we eliminate these? That is, can we rewrite that code to leverage the u
Mmmm possibly yes.
http://gerrit.cloudera.org:8080/#/c/4654/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
PS1, Line 1060: request.errback(e);
: LOG.debug("Cannot continue with this RPC: {} because of:
{}", request, message, e);
: return Deferred.fromError(e);
> What's the point of both returning a Deferred and invoking request.errback(
Right, related to your comment on the commit message, if we just relied on the
RPC's own Deferred, we wouldn't need to return one here and that'd be it.
Line 1261: private Deferred delayedSendRpcToTablet(final KuduRpc
rpc, KuduException ex) {
> Even though it's private, you should document it to explain the semantic me
Done
--
To view, visit http://gerrit.cloudera.org:8080/4654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-HasComments: Yes
[kudu-CR] [java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
Jean-Daniel Cryans has uploaded a new patch set (#2).
Change subject: [java client] AsyncKuduClient#delayedSendRpcToTablet should
return a Deferred
..
[java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
There's some confusion in the Java client code regarding where Deferred objects
are returned and also sent via an RPC's callback. Such a place is
delayedSendRpcToTablet,
since it's called from both the context of paths that come from the user
application (need a Deferred) and paths that simply handle background retries
(don't
need a Deferred since we rely on KuduRpc#callback).
This problem manifested itself in a unit test:
02:15:40.300 [DEBUG - main] (AsyncKuduClient.java:1065) Cannot continue with
this RPC: Batch{operations=1, tablet="4114911c600b4bc3bbca228f1880568c"
[, ), ignoreAllDuplicateRows=false} because of: RPC can not
complete before timeout:
org.apache.kudu.client.NonRecoverableException: RPC can not complete before
timeout: Batch{operations=1, tablet="4114911c600b4bc3bbca228f1880568c"
[, ), ignoreAllDuplicateRows=false}
at
org.apache.kudu.client.AsyncKuduClient.tooManyAttemptsOrTimeout(AsyncKuduClient.java:1063)
at
org.apache.kudu.client.AsyncKuduClient.delayedSendRpcToTablet(AsyncKuduClient.java:1275)
at
org.apache.kudu.client.AsyncKuduClient.sendRpcToTablet(AsyncKuduClient.java:756)
at
org.apache.kudu.client.AsyncKuduSession$TabletLookupCB.call(AsyncKuduSession.java:371)
at
org.apache.kudu.client.AsyncKuduSession$TabletLookupCB.call(AsyncKuduSession.java:302)
at com.stumbleupon.async.Deferred.doCall(Deferred.java:1280)
at com.stumbleupon.async.Deferred.addCallbacks(Deferred.java:685)
at com.stumbleupon.async.Deferred.addBoth(Deferred.java:769)
at org.apache.kudu.util.AsyncUtil.addBoth(AsyncUtil.java:59)
at
org.apache.kudu.client.AsyncKuduSession.doFlush(AsyncKuduSession.java:436)
at
org.apache.kudu.client.AsyncKuduSession.flush(AsyncKuduSession.java:405)
at
org.apache.kudu.client.TestAsyncKuduSession.testInsertIntoUnavailableTablet(TestAsyncKuduSession.java:164)
Here the Deferred.fromError that got created in tooManyAttemptsOrTimeout was
ignored by delayedSendRpcToTablet, then
in sendRpcToTablet we sent back the RPC's Deferred which at that point was
voided since tooManyAttemptsOrTimeout
_also_ called that RPC's callback.
I tried writing a test, but even with looping on my machine I can't repro the
issue.
Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 18 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/4654/2
--
To view, visit http://gerrit.cloudera.org:8080/4654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR](gh-pages) Update docs from branch-1.0.x
Jean-Daniel Cryans has posted comments on this change. Change subject: Update docs from branch-1.0.x .. Patch Set 7: Code-Review+2 Let's release this! -- To view, visit http://gerrit.cloudera.org:8080/4681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58c2d44b2e753db76485f33c496a06a919b2e0c5 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Update docs from branch-1.0.x
Jean-Daniel Cryans has posted comments on this change. Change subject: Update docs from branch-1.0.x .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/4681/6/releases/1.0.1/index.md File releases/1.0.1/index.md: PS6, Line 2: 1.0.0 nit -- To view, visit http://gerrit.cloudera.org:8080/4681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58c2d44b2e753db76485f33c496a06a919b2e0c5 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [doc] added link to online Kudu C++ client API
Jean-Daniel Cryans has posted comments on this change. Change subject: [doc] added link to online Kudu C++ client API .. Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/4684/3/docs/installation.adoc File docs/installation.adoc: Line 236: [[build_from_source]] not needed PS3, Line 637: nit: extra blank space PS3, Line 637: build_from_source use _build_from_source PS3, Line 639: nit: by* opening PS3, Line 639: nit: the* locally PS3, Line 643: installated nit: installed* PS3, Line 643: nit: extra blank space PS3, Line 645: the nit: remove -- To view, visit http://gerrit.cloudera.org:8080/4684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4d77b9b154b635c4aedaced9b050942529cf65f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Update docs from branch-1.0.x
Jean-Daniel Cryans has posted comments on this change. Change subject: Update docs from branch-1.0.x .. Patch Set 2: What about the releases page? https://github.com/apache/kudu/tree/gh-pages/releases -- To view, visit http://gerrit.cloudera.org:8080/4681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58c2d44b2e753db76485f33c496a06a919b2e0c5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Add weekly post for 10/11
Jean-Daniel Cryans has posted comments on this change. Change subject: Add weekly post for 10/11 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic843922a5c77a9493a15c6d8e4c8040fed7a2e83 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred .. Patch Set 1: Verified+1 python test leakage -- To view, visit http://gerrit.cloudera.org:8080/4654 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] [java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
Jean-Daniel Cryans has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4654
Change subject: [java client] AsyncKuduClient#delayedSendRpcToTablet should
return a Deferred
..
[java client] AsyncKuduClient#delayedSendRpcToTablet should return a Deferred
There's some confusion in the Java client code regarding where Deferred objects
are returned and also sent via an RPC's callback. Such a place is
delayedSendRpcToTablet,
since it's called from both the context of paths that come from the user
application (need a Deferred) and paths that simply handle background retries
(don't
need a Deferred since we rely on KuduRpc#callback).
This problem manifested itself in a unit test:
02:15:40.300 [DEBUG - main] (AsyncKuduClient.java:1065) Cannot continue with
this RPC: Batch{operations=1, tablet="4114911c600b4bc3bbca228f1880568c"
[, ), ignoreAllDuplicateRows=false} because of: RPC can not
complete before timeout:
org.apache.kudu.client.NonRecoverableException: RPC can not complete before
timeout: Batch{operations=1, tablet="4114911c600b4bc3bbca228f1880568c"
[, ), ignoreAllDuplicateRows=false}
at
org.apache.kudu.client.AsyncKuduClient.tooManyAttemptsOrTimeout(AsyncKuduClient.java:1063)
at
org.apache.kudu.client.AsyncKuduClient.delayedSendRpcToTablet(AsyncKuduClient.java:1275)
at
org.apache.kudu.client.AsyncKuduClient.sendRpcToTablet(AsyncKuduClient.java:756)
at
org.apache.kudu.client.AsyncKuduSession$TabletLookupCB.call(AsyncKuduSession.java:371)
at
org.apache.kudu.client.AsyncKuduSession$TabletLookupCB.call(AsyncKuduSession.java:302)
at com.stumbleupon.async.Deferred.doCall(Deferred.java:1280)
at com.stumbleupon.async.Deferred.addCallbacks(Deferred.java:685)
at com.stumbleupon.async.Deferred.addBoth(Deferred.java:769)
at org.apache.kudu.util.AsyncUtil.addBoth(AsyncUtil.java:59)
at
org.apache.kudu.client.AsyncKuduSession.doFlush(AsyncKuduSession.java:436)
at
org.apache.kudu.client.AsyncKuduSession.flush(AsyncKuduSession.java:405)
at
org.apache.kudu.client.TestAsyncKuduSession.testInsertIntoUnavailableTablet(TestAsyncKuduSession.java:164)
Here the Deferred.fromError that got created in tooManyAttemptsOrTimeout was
ignored by delayedSendRpcToTablet, then
in sendRpcToTablet we sent back the RPC's Deferred which at that point was
voided since tooManyAttemptsOrTimeout
_also_ called that RPC's callback.
I tried writing a test, but even with looping on my machine I can't repro the
issue.
Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 8 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/4654/1
--
To view, visit http://gerrit.cloudera.org:8080/4654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) .. KUDU-1669. Java client tests leak orphan processes (part 2) In some cases we weren't waiting for the processes to actually finish. Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Reviewed-on: http://gerrit.cloudera.org:8080/4636 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java 1 file changed, 27 insertions(+), 9 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java ITClient test can orphan processes
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-1669. Java ITClient test can orphan processes .. KUDU-1669. Java ITClient test can orphan processes This removes the interruption of threads, which is the reason why we were orphaning processes (in the ChaosThread). Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b Reviewed-on: http://gerrit.cloudera.org:8080/4598 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 2 insertions(+), 3 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Retry regressing counts in ITClient
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Retry regressing counts in ITClient .. [java client] Retry regressing counts in ITClient There's currently no sure way to read your writes, even with snapshot scans, so we can either retry counting rows or ignore it. This patch is doing the former, unless we hit some artificial timeout. Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Reviewed-on: http://gerrit.cloudera.org:8080/4597 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 26 insertions(+), 12 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4636/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java: Line 363: } > Cool, so this worked out? Might want to add a comment explaining what's goi Done. -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4636 to look at the new patch set (#3). Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) .. KUDU-1669. Java client tests leak orphan processes (part 2) In some cases we weren't waiting for the processes to actually finish. Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java 1 file changed, 27 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4636/3 -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java ITClient test can orphan processes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4598 to look at the new patch set (#3). Change subject: KUDU-1669. Java ITClient test can orphan processes .. KUDU-1669. Java ITClient test can orphan processes This removes the interruption of threads, which is the reason why we were orphaning processes (in the ChaosThread). Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/3 -- To view, visit http://gerrit.cloudera.org:8080/4598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4636 to look at the new patch set (#2). Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) .. KUDU-1669. Java client tests leak orphan processes (part 2) In some cases we weren't waiting for the processes to actually finish. Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java 1 file changed, 23 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4636/2 -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Retry regressing counts in ITClient
Hello David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4597 to look at the new patch set (#4). Change subject: [java client] Retry regressing counts in ITClient .. [java client] Retry regressing counts in ITClient There's currently no sure way to read your writes, even with snapshot scans, so we can either retry counting rows or ignore it. This patch is doing the former, unless we hit some artificial timeout. Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 26 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/4597/4 -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-1669. Java client tests leak orphan processes (part 2)
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4636/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java:
PS1, Line 359: catch (InterruptedException ex) {
: // We still need to finish cleaning up.
: }
> This isn't quite correct; if we are interrupted (not that we should be, but
I guess if we really really want to be clean then yeah we'll have to wrap those
individually. As you can see, I didn't want to go there.
Line 363: thread.interrupt();
> It would be nice to avoid using thread interruption here too. I wonder: if
Checking...
--
To view, visit http://gerrit.cloudera.org:8080/4636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Retry regressing counts in ITClient
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Retry regressing counts in ITClient
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4597/3/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java:
Line 386: do {
> Retaining the printout doesn't change the flow in a substantial way:
Yup I wrote exactly that in the change I'm about to push. Like I said
originally, this whole thing is messier and the gain from retrying is ~0.
--
To view, visit http://gerrit.cloudera.org:8080/4597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Rename NoLeaderMasterFoundException and reuse
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Rename NoLeaderMasterFoundException and reuse .. [java client] Rename NoLeaderMasterFoundException and reuse I pushed d87486c too fast, Dan convinced me to reuse the exception. Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 Reviewed-on: http://gerrit.cloudera.org:8080/4624 Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/GetMasterRegistrationReceived.java R java/kudu-client/src/main/java/org/apache/kudu/client/NoLeaderFoundException.java D java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestGetMasterRegistrationReceived.java 6 files changed, 14 insertions(+), 46 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Retry regressing counts in ITClient
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Retry regressing counts in ITClient
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4597/3/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java:
Line 386: do {
> Nit: the loop structure is unusual in that the normal control flow takes us
I see you don't like my taste :) Sure I can give it a try. I did like having
the printout for new row count though, really helps when debugging, and putting
it back makes your solution slightly less pretty.
--
To view, visit http://gerrit.cloudera.org:8080/4597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Retry regressing counts in ITClient
Hello David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4597 to look at the new patch set (#3). Change subject: [java client] Retry regressing counts in ITClient .. [java client] Retry regressing counts in ITClient There's currently no sure way to read your writes, even with snapshot scans, so we can either retry counting rows or ignore it. This patch is doing the former, unless we hit some artificial timeout. Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 26 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/4597/3 -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java ITClient test can orphan processes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4598 to look at the new patch set (#2). Change subject: KUDU-1669. Java ITClient test can orphan processes .. KUDU-1669. Java ITClient test can orphan processes This removes the interruption of threads, which is the reason why we were orphaning processes (in the ChaosThread). Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/2 -- To view, visit http://gerrit.cloudera.org:8080/4598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1669. Java client tests leak orphan processes (part 2)
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4636 Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) .. KUDU-1669. Java client tests leak orphan processes (part 2) In some cases we weren't waiting for the processes to actually finish. Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java 1 file changed, 19 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/4636/1 -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] maintenance manager-test: fix flaky test
Jean-Daniel Cryans has posted comments on this change. Change subject: maintenance_manager-test: fix flaky test .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I003e72339f69228195130c89572a20be3009f22c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Temporarily ignore row count regressions in ITClient
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Temporarily ignore row count regressions in ITClient .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4597/2/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java: Line 383: int rowCount; Adar, I did the looping thing. -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [java client] Temporarily ignore row count regressions in ITClient
Hello David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4597 to look at the new patch set (#2). Change subject: [java client] Temporarily ignore row count regressions in ITClient .. [java client] Temporarily ignore row count regressions in ITClient There's currently no sure way to read your writes, even with snapshot scans, so we can either retry counting rows or ignore it. This patch is doing the former, unless we hit some artificial timeout. Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 26 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/4597/2 -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Rename NoLeaderMasterFoundException and reuse
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Rename NoLeaderMasterFoundException and reuse .. Patch Set 1: > I'm assuming you want to keep around NoSuitableReplicaException for > future use? Argh no, forgot to delete. -- To view, visit http://gerrit.cloudera.org:8080/4624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Rename NoLeaderMasterFoundException and reuse
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4624 to look at the new patch set (#2). Change subject: [java client] Rename NoLeaderMasterFoundException and reuse .. [java client] Rename NoLeaderMasterFoundException and reuse I pushed d87486c too fast, Dan convinced me to reuse the exception. Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/GetMasterRegistrationReceived.java R java/kudu-client/src/main/java/org/apache/kudu/client/NoLeaderFoundException.java D java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestGetMasterRegistrationReceived.java 6 files changed, 14 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/4624/2 -- To view, visit http://gerrit.cloudera.org:8080/4624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Rename NoLeaderMasterFoundException and reuse
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4624 Change subject: [java client] Rename NoLeaderMasterFoundException and reuse .. [java client] Rename NoLeaderMasterFoundException and reuse I pushed d87486c too fast, Dan convinced me to reuse the exception Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/GetMasterRegistrationReceived.java R java/kudu-client/src/main/java/org/apache/kudu/client/NoLeaderFoundException.java M java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestGetMasterRegistrationReceived.java 6 files changed, 19 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/4624/1 -- To view, visit http://gerrit.cloudera.org:8080/4624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I51ad20943fe8b4b808123dc8b7795215f3b12308 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. Patch Set 3: Verified+1 Dan messed up my build, but I did run all the Java tests locally before pushing the latest patchset. -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader There's currently the possibility of a situation like this: 1. sendRpcToTablet finds the tablet that the RPC is going to doesn't have a leader 2. a master lookup is sent 3. discoverTablet is called back and still doesn't find a leader 4. RetryRpcCallback is invoked right away, going back to step 1 This quickly gets us with this exception: Too many attempts: KuduRpc(method=Write, tablet=redacted, attempt=101, DeadlineTracker(timeout=3, elapsed=4747) Notice how it retried 101 times in less than 5 seconds. This patch changes step 3 so that an exception is thrown so that RetryRpcErrback is invoked instead, which will add delay before retrying the RPC. This bug was found by ITClient. It's not _just_ a flaky test! Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Reviewed-on: http://gerrit.cloudera.org:8080/4570 Reviewed-by: Adar Dembo Tested-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java 3 files changed, 100 insertions(+), 19 deletions(-) Approvals: Jean-Daniel Cryans: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4570 to look at the new patch set (#3). Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader There's currently the possibility of a situation like this: 1. sendRpcToTablet finds the tablet that the RPC is going to doesn't have a leader 2. a master lookup is sent 3. discoverTablet is called back and still doesn't find a leader 4. RetryRpcCallback is invoked right away, going back to step 1 This quickly gets us with this exception: Too many attempts: KuduRpc(method=Write, tablet=redacted, attempt=101, DeadlineTracker(timeout=3, elapsed=4747) Notice how it retried 101 times in less than 5 seconds. This patch changes step 3 so that an exception is thrown so that RetryRpcErrback is invoked instead, which will add delay before retrying the RPC. This bug was found by ITClient. It's not _just_ a flaky test! Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java 3 files changed, 100 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/4570/3 -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Tight-ish loop in master lookups if a tablet
doesn't have a leader
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4570/2/java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java:
Line 23: final class NoSuitableReplicaException extends RecoverableException {
> How about renaming NoLeaderMasterFoundException to NoLeaderFoundException a
It's not really the same thing, I think. There could be other reasons why we
can't find a suitable replica, although at the moment we only have the missing
leader situation.
http://gerrit.cloudera.org:8080/#/c/4570/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java:
PS2, Line 196: insert.partitionKey()
> this could just be the empty array, no need to create an insert.
Done
--
To view, visit http://gerrit.cloudera.org:8080/4570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4570/1//COMMIT_MSG Commit Message: PS1, Line 19: 101 > It's useful for development, yes, but not useful (and I would argue potenti Not having this limit would be hurtful in production, it means runaway operations can start hammering master and tablet servers as long as their timeout allows it. If the bug this jira is fixing happened in prod, you'd hit the master with GetTabletLocations as often as you can for 30 seconds. http://gerrit.cloudera.org:8080/#/c/4570/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 1498: // TODO: Handle the situation when multiple in-flight RPCs are queued waiting > Hmm, maybe we should acquire a "master lookup permit" here then? Not releva Yup. -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-1563. Add support for INSERT IGNORE
Jean-Daniel Cryans has posted comments on this change. Change subject: [java] KUDU-1563. Add support for INSERT IGNORE .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib0cc4a533dfb01a883d347c9795c165aa8efa3fd Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Brock Noland Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Separate tablet MM ops into separate .cc file
Jean-Daniel Cryans has posted comments on this change. Change subject: Separate tablet MM ops into separate .cc file .. Patch Set 6: Code-Review+2 I didn't run a line-by-line comparison of your copy paste so I assume it's fine :) -- To view, visit http://gerrit.cloudera.org:8080/4362 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie40475fdb9f5da6a7300661d5149509da990939d Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] java: fix kudu-client builds with TSAN-only thirdparty
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: java: fix kudu-client builds with TSAN-only thirdparty .. java: fix kudu-client builds with TSAN-only thirdparty This turned out to be somewhat messy as only newer versions of maven[1] allow profile activation when multiple conditions are true. I tested this via repeated calls to "mvn help:active-profiles" and "mvn clean compile" while hiding various combinations of my tsan, uninstrumented, and system protoc executables. 1. https://issues.apache.org/jira/browse/MNG-4565 Change-Id: I580d69507abf0e486fc8f3f364177d0a60b6382b Reviewed-on: http://gerrit.cloudera.org:8080/4577 Tested-by: Adar Dembo Reviewed-by: Jean-Daniel Cryans --- M java/kudu-client/pom.xml 1 file changed, 67 insertions(+), 15 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Adar Dembo: Verified -- To view, visit http://gerrit.cloudera.org:8080/4577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I580d69507abf0e486fc8f3f364177d0a60b6382b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] java: fix kudu-client builds with TSAN-only thirdparty
Jean-Daniel Cryans has posted comments on this change. Change subject: java: fix kudu-client builds with TSAN-only thirdparty .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I580d69507abf0e486fc8f3f364177d0a60b6382b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) [java client] make DateFormat safe to use
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] make DateFormat safe to use .. [java client] make DateFormat safe to use Todd's error-prone patch discovered this problem in RowResult where we use a static DateFormat from multiple threads. The recommended way to do this is to use thread locals. Change-Id: I6d18ba34db0a0782fd0b7401e9aea7ae59c6b6f6 Reviewed-on: http://gerrit.cloudera.org:8080/4429 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit b0b273e8271752b6eb04ba163981aad1c792e413) Reviewed-on: http://gerrit.cloudera.org:8080/4603 Reviewed-by: Jean-Daniel Cryans Tested-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java 1 file changed, 12 insertions(+), 5 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4603 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6d18ba34db0a0782fd0b7401e9aea7ae59c6b6f6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR](branch-1.0.x) [java client] make DateFormat safe to use
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] make DateFormat safe to use .. Patch Set 1: Code-Review+2 Verified+1 Unrelated C++ test failure. -- To view, visit http://gerrit.cloudera.org:8080/4603 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d18ba34db0a0782fd0b7401e9aea7ae59c6b6f6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) [java client] Fix an NPE in KuduException
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [java client] Fix an NPE in KuduException .. [java client] Fix an NPE in KuduException Saw this in a Jenkins run and also running ITClient on my machine. Change-Id: Iceddc6931e8d3a8cb807657fc5c0804f7052e48f Reviewed-on: http://gerrit.cloudera.org:8080/4488 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins (cherry picked from commit 8fc75a5c654e100871316e61878b141df4707d0e) Reviewed-on: http://gerrit.cloudera.org:8080/4605 Reviewed-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java 1 file changed, 6 insertions(+), 3 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iceddc6931e8d3a8cb807657fc5c0804f7052e48f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.0.x) [java client] Fix an NPE in KuduException
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Fix an NPE in KuduException .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iceddc6931e8d3a8cb807657fc5c0804f7052e48f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Temporarily ignore row count regressions in ITClient
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Temporarily ignore row count regressions in
ITClient
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4597/1/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java:
Line 389: LOG.warn("Row count regressed: " + rowCount + " < " +
lastRowCount);
> maybe I'm missing some context, but is there a way to loop instead of ignor
Like I'm saying in the commit message, looping until we get a correct count
would be kind of messy. Not sure if it actually helps with anything TBH.
--
To view, visit http://gerrit.cloudera.org:8080/4597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1669. Java ITClient test can orphan processes
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4598 Change subject: KUDU-1669. Java ITClient test can orphan processes .. KUDU-1669. Java ITClient test can orphan processes ITClient#configureAndStartProcess now catches the InterruptedException and cleans up the newly created process. Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java 1 file changed, 16 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/1 -- To view, visit http://gerrit.cloudera.org:8080/4598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Temporarily ignore row count regressions in ITClient
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4597 Change subject: [java client] Temporarily ignore row count regressions in ITClient .. [java client] Temporarily ignore row count regressions in ITClient There's currently no sure way to read your writes, even with snapshot scans, so we can either retry counting rows or ignore it. This patch is doing the latter since it's less messy and hopefully this situation will be resolved soon. Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 --- M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/4597/1 -- To view, visit http://gerrit.cloudera.org:8080/4597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1e79a6c7aaf069294a6ca40e487947d14d9f2aa7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has uploaded a new patch set (#2). Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader There's currently the possibility of a situation like this: 1. sendRpcToTablet finds the tablet that the RPC is going to doesn't have a leader 2. a master lookup is sent 3. discoverTablet is called back and still doesn't find a leader 4. RetryRpcCallback is invoked right away, going back to step 1 This quickly gets us with this exception: Too many attempts: KuduRpc(method=Write, tablet=redacted, attempt=101, DeadlineTracker(timeout=3, elapsed=4747) Notice how it retried 101 times in less than 5 seconds. This patch changes step 3 so that an exception is thrown so that RetryRpcErrback is invoked instead, which will add delay before retrying the RPC. This bug was found by ITClient. It's not _just_ a flaky test! Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java 3 files changed, 104 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/4570/2 -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
