[kudu-CR] [java-client] separating Connection
Dan Burkert has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 13: (9 comments) http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG Commit Message: Line 11: The TabletClient has been renamed into RpcHelper. Also, update 'RpcHelper' http://gerrit.cloudera.org:8080/#/c/7146/9/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 151: /** > Just a collection of object representing connections to the servers, no any Hmm so I don't think the new comment is adding much clarity. I think i'd prefer the original, minus the word 'trivial'. My question about 'trivial' is that it suggests the cache is deficient or not-fully feature in some way, but AFAICT it's exactly what we need :) Line 170: > IntellyJIdea was pointing at this as a broken link. This is relative to where you have set up the root IntelliJ project. I'd prefer it you leave it as is, since it's also possible to configure IntelliJ relative to the repo root. http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 74: * a response is currently awaited, as well as temporarily buffered RPCs that are waiting > It seems this comment is stale: there is no need to care about Netty channe Great! Line 598: > Done -- I ended up calling it 'READY'. Good name. http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 479:* @return true iff the connection is in the READY state. omit the trailing period http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java: Line 51: * @{link Connection} objects. I think the @ needs to go inside the opening brace Line 61: /** The logger to use. */ This comment can be omitted, it's obvious from context Line 73:* @param client top-level Kudu client object. omit trailing periods on param docs -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Alexey Serbin has uploaded a new patch set (#13). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcProxy.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,395 insertions(+), 1,266 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/13 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 12: Verified+1 Unrelated flake in client-stress-test -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java-client] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#12). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcProxy.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,385 insertions(+), 1,266 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/12 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 10: (20 comments) http://gerrit.cloudera.org:8080/#/c/7146/9/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 603 > Would this be difficult to keep? It would be better not to remove it since Right, I returned it back already. No, it's not a problem to keep it. Line 151: /** A trivial cache to keep track of already opened connections to Kudu servers. */ > What does trivial mean in this context? Just a collection of object representing connections to the servers, no any special behavior. I'll update the comment. Line 170:* @see ../src/kudu/common/common.proto > Why this change? IntellyJIdea was pointing at this as a broken link. Line 238: > Add a method doc. Done Line 754: closeRequest.attempt++; > What's the significance of the ArrayList in the return type? Could it just Nothing particular, just propagated the result from disconnectEverything(). Yes, it can be just Deferred. Also, this method is used only in tests, so I'll just remove it and replace with { getConnection(), disconnectEverything() } combination. Line 934:* the provided callback will be called. > Wrap the second arg Done http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: PS10, Line 70: RPC > RPCs Done PS10, Line 71: awaiting > waiting Done Line 74: * This class needs careful synchronization. It's a non-sharable handler, > Could you make this paragraph more precise by calling out that 'everything It seems this comment is stale: there is no need to care about Netty channel anymore since it's managed internally. Also, synchronization is already taken care of in other places, and I fixed the issue with the disconnect() method. Line 84: */ > yea I consider the use of socket timeouts to be a bug rather than a feature ok, I added TODO Line 246: ArrayList queued; > This can be a List Done Line 254: queued = queuedMessages; > The declaration of 'queued' can be moved here. Done Line 320: "Tablet server sent error " + error.getMessage(); > This should probably be 'Server sent error' Done Line 536: ArrayList queued; > This can be a List Done Line 537: HashMap> inflight; > This can be a Map<..> Done Line 568: inflight.clear(); > Is this necessary? Good catch -- no, it's not necessary. It seems I just re-did the original implementation of the loop where the elements were removed from the container while iterating over them. Line 598: NEGOTIATED, // The connection has negotiated and ready to use. > I think this state would be clearer if it was called 'Open' or 'Running' or Done -- I ended up calling it 'READY'. http://gerrit.cloudera.org:8080/#/c/7146/10/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: PS10, Line 36: Kudu server-side components > I think it was clearer as 'masters and tablet servers' Done http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java: Line 49: * This is a stateless helper to send RPCs to a Kudu server. > It doesn't appear to be stateless, although it could easily be made so. Pe It's stateless in the sense that it does not store any state in it, just references to the AsyncKuduClient and Connection instances. There would be more methods than one if making it static, I think. I looked at that approach and in some cases it would require passing both connection and Kudu client reference, like in ConnectToCluster.java. That's why I decided to keep it like this. Line 57: public class RpcHelper { > Is this the same as the Proxy on the c++ side? Not exactly, but close. Todd was not happy with the name for this class, so I'll rename it into RpcProxy. -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#11). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,381 insertions(+), 1,266 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/11 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Jean-Daniel Cryans has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 84: */ > yea I consider the use of socket timeouts to be a bug rather than a feature Not aware of a jira. The ReadTimeoutHandler was the only thing I found in Netty that was a proxy for a socket timeout. Of course it's very coarse grained and Todd hates it. -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Todd Lipcon has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 84: */ > Could you add a comment to the class explaining why it's using a socket rea yea I consider the use of socket timeouts to be a bug rather than a feature... timeouts should be call-scoped rather than something on the socket level. but I dont think it's within scope of this patch to fix that. maybe a TODO? I thought we had a JIRA about this but I can't find it now - maybe JD knows. http://gerrit.cloudera.org:8080/#/c/7146/9/java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java: PS9, Line 57: RpcHelper nit: I have a bit of a pet peeve about classes named "Helper" because it's not very illustrative about what the class actually does. Maybe this is 'OutboundRpc' or 'RpcRouter' or 'RpcRetrier' or 'InFlightRpc' or something like that? (still have to review this patch in whole so maybe I'll have a better idea for a name after doing so!) -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Dan Burkert has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 10: (20 comments) http://gerrit.cloudera.org:8080/#/c/7146/9/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 603 Would this be difficult to keep? It would be better not to remove it since it's in the public API (even thought this whole class is unstable). Line 151: /** A trivial cache to keep track of already opened connections to Kudu servers. */ What does trivial mean in this context? Line 170:* @see ../src/kudu/common/common.proto Why this change? Line 238: Add a method doc. Line 754: closeRequest.attempt++; What's the significance of the ArrayList in the return type? Could it just be Deferred ? Line 934:* the provided callback will be called. Wrap the second arg http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: PS10, Line 70: RPC RPCs PS10, Line 71: awaiting waiting Line 74: * This class needs careful synchronization. It's a non-sharable handler, Could you make this paragraph more precise by calling out that 'everything else' is marked with @GuardedBy annotations. Line 84: */ Could you add a comment to the class explaining why it's using a socket read timeout? That detail is different than C++, and I frequently can't remember why it differs. Line 246: ArrayList queued; This can be a List Line 254: queued = queuedMessages; The declaration of 'queued' can be moved here. Line 320: "Tablet server sent error " + error.getMessage(); This should probably be 'Server sent error' Line 536: ArrayList queued; This can be a List Line 537: HashMap> inflight; This can be a Map<..> Line 568: inflight.clear(); Is this necessary? Line 598: NEGOTIATED, // The connection has negotiated and ready to use. I think this state would be clearer if it was called 'Open' or 'Running' or something like that. http://gerrit.cloudera.org:8080/#/c/7146/10/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: PS10, Line 36: Kudu server-side components I think it was clearer as 'masters and tablet servers' http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java: Line 49: * This is a stateless helper to send RPCs to a Kudu server. It doesn't appear to be stateless, although it could easily be made so. Perhaps this class should just expose a single static method which takes the 'client', 'connection' and 'rpc'? Line 57: public class RpcHelper { Is this the same as the Proxy on the c++ side? -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Alexey Serbin has uploaded a new patch set (#10). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,398 insertions(+), 1,256 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/10 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 9: Verified+1 It seems TestKuduClient.testCloseShortlyAfterOpen was flaky before my changes, but I'll take a look at it anyway. -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java-client] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/7146/8/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: PS8, Line 134: private > nit: private comes first. Done Line 207: @Nullable > Seems weird to put these two methods before the ctor. Done PS8, Line 537: repeatably > Typo? Done -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#9). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,399 insertions(+), 1,268 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/9 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Jean-Daniel Cryans has posted comments on this change. Change subject: [java-client] separating Connection .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/7146/8/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: PS8, Line 134: private nit: private comes first. Line 207: @Nullable Seems weird to put these two methods before the ctor. PS8, Line 537: repeatably Typo? -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#8). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,384 insertions(+), 1,252 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/8 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#7). Change subject: [java-client] separating Connection .. [java-client] separating Connection This patch separates connection-related stuff from the TabletClient class. The TabletClient has been renamed into RpcHelper. Also, this patch brings in other micro clean-ups in the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.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/Negotiator.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/RpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.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 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 M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,407 insertions(+), 1,285 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/7 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon