You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/06/10 06:59:39 UTC
[kudu-CR] WIP [java-client] separating Connection
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7146
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary to add.
Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
---
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/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.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/BaseKuduTest.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/TestConnectionCache.java
9 files changed, 795 insertions(+), 611 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/1
--
To view, visit http://gerrit.cloudera.org:8080/7146
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, 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 (#18).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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,401 insertions(+), 1,264 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/18
--
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#15).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated 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,396 insertions(+), 1,252 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/15
--
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: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#16).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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,401 insertions(+), 1,264 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/16
--
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: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: WIP [java-client] separating Connection
......................................................................
Patch Set 2:
(13 comments)
Thank you for the review.
I updated the patch, addressing the feedback and moving the uuid2client container into AsyncKuduClient from ConnectionCache.
The next step is to get rid of uuid2client at all -- I'm thinking to make all the methods of the TabletClient static, and adding Connection and AsyncKuduClient as parameters. The exclusive caller of the TabletClient is AsyncKuduClient (not counting TestConnectionCache).
http://gerrit.cloudera.org:8080/#/c/7146/2/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:
PS2, Line 104: /**
: * The class to represent RPC response received from the remote server.
: * If the @exception is null, then it's a success case, otherwise it's an error.
: * For the recoverable error case, the @exception is of RecoverableException type,
: * otherwise it's of NonRecoverableException type.
: */
: static final class CallResponseInfo {
: public final CallResponse response;
: public final KuduException exception;
:
: CallResponseInfo(CallResponse response, KuduException exception) {
: this.response = response;
: this.exception = exception;
: }
: }
> nit: it's a little weird to have this class definition (and the one below)
Done
PS2, Line 124: queuedMessages
> some typo here
Done
PS2, Line 125: private final class QueuedMessage {
> should this be static?
Done
Line 136: private ArrayList<QueuedMessage> queuedMessages = Lists.newArrayList();
> add javadoc
Done
PS2, Line 138: /** Read timeout for the connection (used by Netty's ReadTimeoutHandler) */
: private final long socketReadTimeoutMs;
: /** Timer to monitor read timeouts for the connection (used by Netty's ReadTimeoutHandler) */
: private final HashedWheelTimer timer;
:
: /** Security context to use for connection negotiation. */
: private final SecurityContext securityContext;
> would be nice to reorder the final fields separately from those fields that
Done
PS2, Line 188: lock.lock();
> nit: this usually goes outside of the 'try' so that if for whatever reason
woops, that a typo, thanks.
PS2, Line 198: Nullable
> I wonder if we could actually make this illegal to call prior to a connecti
Yep, that would be nice, but right now I don't see how we can do it since the outgoing messages can be queued even if when connection is not yet established.
I'll add TODO for addressing this later.
PS2, Line 241: super.channelDisconnected(ctx, e); // Let the ReplayingDecoder cleanup.
> i think the comment here is stale, we don't use ReplayingDecoder anymore. D
Yep, the comment is stale.
The SimpleChannelUpstreamHandler.channelDisconnected() calls ctx.sendUpstream() for this event, but I don't see anybody higher upstream who might be interested handling this event.
It looks safe to remove the call to super.channelDisconnected() here and the call to super.channelClosed() in channelClosed() -- we build the Netty's pipeline ourselves and don't expose the underlying Netty entities outside.
PS2, Line 263: queuedMessages = Lists.newArrayList();
> once we are in NEGOTIATED state, is queuedMessages ever used again? I wonde
good catch -- yes, it's possible to do so.
PS2, Line 270: sendCallToWire(qm.message, qm.cb);
> sendCallToWire says it's GuardedBy("lock") but here you've released it alre
good catch; fixed
PS2, Line 378: // Schedule (re)connecting to the server.
: connect();
: }
> in your plans for a refactor are you going to attempt to simplify this life
It's a great observation. Yes, I think simpler connection recycling is the best option here.
Done.
http://gerrit.cloudera.org:8080/#/c/7146/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java:
PS2, Line 24: queuedMessages
> errant find/replace?
Done
http://gerrit.cloudera.org:8080/#/c/7146/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
Line 134: * @return true if operations are queuedMessages, else false.
> other errant find/replaces in this file
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 21: Code-Review+2
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 21: Verified+1
Unrelated flake RaftConsensusITest.TestChurnyElections
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#5).
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient. In
addition, it addresses KUDU-1878.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary.
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/Negotiator.java
M 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/BaseKuduTest.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/TestConnectionCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
11 files changed, 1,117 insertions(+), 854 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/5
--
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: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 18:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 228: helper
nit: here and below it says 'helper' instead of 'proxy' (leftover from an earlier revision)
PS18, Line 250: as
sentence ends abruptly here
PS18, Line 730: Not connected to server, will retry later
is this error message still accurate? when would the above return null? it no longer seems to be related to a connection-cache miss
http://gerrit.cloudera.org:8080/#/c/7146/18/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java:
PS18, Line 84: masterHelper
rename to masterProxy
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 71: connection
a connection
PS18, Line 73: RPCs in flights
RPCs in flight
PS18, Line 86: public
odd to see this one public -- is it visible for testing? or should we have a getter?
Line 110: /** Initial header sent by the client upon connection establishment. */
nit: add newline before this
PS18, Line 349: negotiationResult
this should be guarded by 'lock' right?
PS18, Line 578: @response
same
PS18, Line 578: @exception
this is odd syntax for javadoc
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 52: the connection
new connections?
PS18, Line 55: connection
connections?
PS18, Line 110: messages
RPCs
Line 132: List<Connection> getConnectionListCopy() {
maybe @VisibleForTesting?
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 134: byte
while we are at it, let's make this an int?
http://gerrit.cloudera.org:8080/#/c/7146/18/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
PS18, Line 158: disconnect or shutdown client connection
update this?
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 478: for (Iterator<Process> it = tserverProcesses.values().iterator(); it.hasNext(); ) {
: final Process p = it.next();
: while (true) {
: try {
: destroyAndWaitForProcess(p);
: break;
: } catch (InterruptedException e) {
: wasInterrupted = true;
: // Not being polite here: ignore the request to interrupt and continue cleaning up.
: LOG.info("ignoring request to interrupt; waiting for tablet server {} to exit", p);
: }
: }
: }
: tserverProcesses.clear();
can we extract a function for this repeated block?
--
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#3).
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient. In
addition, it addresses KUDU-1878.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary to add.
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/Negotiator.java
M 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/BaseKuduTest.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/TestConnectionCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
11 files changed, 1,109 insertions(+), 854 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/3
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#2).
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary to add.
Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
---
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/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.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/BaseKuduTest.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/TestConnectionCache.java
9 files changed, 775 insertions(+), 611 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/2
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#14).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated 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,389 insertions(+), 1,256 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/14
--
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: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#22).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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,504 insertions(+), 1,333 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/22
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 23: -Code-Review Verified+1
Unreated flake in client_samples-test: it seems the master didn't start up or was killed.
--
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: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, 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 (#19).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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,505 insertions(+), 1,336 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/19
--
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: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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
Reviewed-on: http://gerrit.cloudera.org:8080/7146
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
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,504 insertions(+), 1,333 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/7146
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 21:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7146/21/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 731: String.format("No information on servers hosting tablet %s, will retry later",
> does this take care of triggering it to get looked up? Is this path covered
As I understand, the lookup will be performed by the delayed sendRpcToTablet() method call: that's in the very end of the AsyncKuduClient.sendRpcToTablet() implementation block.
I think the path is covered by existing tests, e.g. ITFaultTolerantScanner and ITNonFaultTolerantScanner. At least, when I ran them, I saw they came through this point.
Do you think we need to add some dedicated test here as well?
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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<Void> ?
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<QueuedMessage> queued;
This can be a List<QueuedMessage>
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<QueuedMessage> queued;
This can be a List<QueuedMessage>
Line 537: HashMap<Integer, Callback<Void, CallResponseInfo>> 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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#6).
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient. In
addition, it addresses KUDU-1878.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary.
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/Negotiator.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/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/BaseKuduTest.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/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
15 files changed, 1,135 insertions(+), 958 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/6
--
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 21:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7146/21/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 731: String.format("No information on servers hosting tablet %s, will retry later",
does this take care of triggering it to get looked up? Is this path covered by any tests?
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 23: -Code-Review Verified+1
Unreated flake in client_samples-test: it seems the master didn't start up or was killed.
--
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: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 21:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7146/21/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 731: String.format("No information on servers hosting tablet %s, will retry later",
> As I understand, the lookup will be performed by the delayed sendRpcToTable
k, just wanted to make sure it's actually covered
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7146
to look at the new patch set (#4).
Change subject: WIP [java-client] separating Connection
......................................................................
WIP [java-client] separating Connection
This patch separates connection-related stuff from TabletClient. In
addition, it addresses KUDU-1878.
This work is done in the context of KUDU-2013.
WIP: more clean-up and comments are necessary.
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/Negotiator.java
M 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/BaseKuduTest.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/TestConnectionCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
11 files changed, 1,117 insertions(+), 854 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/4
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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<Void>.
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<QueuedMessage> queued;
> This can be a List<QueuedMessage>
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<QueuedMessage> queued;
> This can be a List<QueuedMessage>
Done
Line 537: HashMap<Integer, Callback<Void, CallResponseInfo>> 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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 22: Code-Review+2
Carrying over Todd's +2 from PS21
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 18:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 228: helper
> nit: here and below it says 'helper' instead of 'proxy' (leftover from an e
Done
PS18, Line 250: as
> sentence ends abruptly here
Done
PS18, Line 730: Not connected to server, will retry later
> is this error message still accurate? when would the above return null? it
Good catch. RemoteTablet will return null if its tabletServers member is empty -- that can happen if RemoteTablet.removeTabletClient() has been called by AsyncKuduClient.invalidateTabletCache() method.
http://gerrit.cloudera.org:8080/#/c/7146/18/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java:
PS18, Line 84: masterHelper
> rename to masterProxy
Done
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 71: connection
> a connection
Done
PS18, Line 73: RPCs in flights
> RPCs in flight
Done
PS18, Line 86: public
> odd to see this one public -- is it visible for testing? or should we have
Yep, probably having a getter is better idea. Done.
Line 110: /** Initial header sent by the client upon connection establishment. */
> nit: add newline before this
Done
PS18, Line 349: negotiationResult
> this should be guarded by 'lock' right?
oh, sure -- good catch!
PS18, Line 578: @response
> same
Done
PS18, Line 578: @exception
> this is odd syntax for javadoc
Done
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 52: the connection
> new connections?
Done
PS18, Line 55: connection
> connections?
Done
PS18, Line 110: messages
> RPCs
Done
Line 132: List<Connection> getConnectionListCopy() {
> maybe @VisibleForTesting?
Well, it's also used by AsyncKuduClient to expose the connection set to tests (so, not only tests use it). But it will not hurt, sure.
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 134: byte
> while we are at it, let's make this an int?
Done
http://gerrit.cloudera.org:8080/#/c/7146/18/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
PS18, Line 158: disconnect or shutdown client connection
> update this?
Done
http://gerrit.cloudera.org:8080/#/c/7146/18/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:
PS18, Line 478: for (Iterator<Process> it = tserverProcesses.values().iterator(); it.hasNext(); ) {
: final Process p = it.next();
: while (true) {
: try {
: destroyAndWaitForProcess(p);
: break;
: } catch (InterruptedException e) {
: wasInterrupted = true;
: // Not being polite here: ignore the request to interrupt and continue cleaning up.
: LOG.info("ignoring request to interrupt; waiting for tablet server {} to exit", p);
: }
: }
: }
: tserverProcesses.clear();
> can we extract a function for this repeated block?
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 15:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG
Commit Message:
Line 11: The updated TabletClient has been renamed into RpcHelper.
> Wasn't it renamed to RpcProxy?
Woops, right. Done.
http://gerrit.cloudera.org:8080/#/c/7146/15/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 464: try {
> Shouldn't this still be a while loop? If it throws interrupted I think it
We discussed this over the kudu-general Slack channel and decided go keep the internal while(true) loop and add also update how the interrupt flag is set on the thread.
--
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: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 23: Code-Review+2 -Verified
Carrying over Todd's +2 from PS21
--
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: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 23: Code-Review+2
Carrying over Todd's +2 from PS21
--
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: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 17: Code-Review+1
LGTM, does Todd or JD want to take another look?
--
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: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 10:
(7 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'
Done
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: /** A trivial cache to keep track of already opened connections to Kudu servers. */
> Hmm so I don't think the new comment is adding much clarity. I think i'd p
Done
Line 170: * @see ../src/kudu/common/common.proto
> This is relative to where you have set up the root IntelliJ project. I'd p
Done
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: }
> omit the trailing period
Done
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
> I think the @ needs to go inside the opening brace
Done
Line 61
> This comment can be omitted, it's obvious from context
Done
Line 73
> omit trailing periods on param docs
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: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, 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 (#21).
Change subject: [java] separating Connection
......................................................................
[java] separating Connection
This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
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,503 insertions(+), 1,333 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/21
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP [java-client] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: WIP [java-client] separating Connection
......................................................................
Patch Set 2:
(13 comments)
did a first pass.
I think many of my comments may equally apply to the pre-refactored code, in which case feel free to just drop TODOs in place for any that you think are worth fixing later.
http://gerrit.cloudera.org:8080/#/c/7146/2/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:
PS2, Line 104: /**
: * The class to represent RPC response received from the remote server.
: * If the @exception is null, then it's a success case, otherwise it's an error.
: * For the recoverable error case, the @exception is of RecoverableException type,
: * otherwise it's of NonRecoverableException type.
: */
: static final class CallResponseInfo {
: public final CallResponse response;
: public final KuduException exception;
:
: CallResponseInfo(CallResponse response, KuduException exception) {
: this.response = response;
: this.exception = exception;
: }
: }
nit: it's a little weird to have this class definition (and the one below) interspersed between member variable definitions
PS2, Line 124: queuedMessages
some typo here
PS2, Line 125: private final class QueuedMessage {
should this be static?
Line 136: private ArrayList<QueuedMessage> queuedMessages = Lists.newArrayList();
add javadoc
PS2, Line 138: /** Read timeout for the connection (used by Netty's ReadTimeoutHandler) */
: private final long socketReadTimeoutMs;
: /** Timer to monitor read timeouts for the connection (used by Netty's ReadTimeoutHandler) */
: private final HashedWheelTimer timer;
:
: /** Security context to use for connection negotiation. */
: private final SecurityContext securityContext;
would be nice to reorder the final fields separately from those fields that are mutable and protected by the lock
PS2, Line 188: lock.lock();
nit: this usually goes outside of the 'try' so that if for whatever reason this threw an exception and didn't acquire a lock, we wouldn't then try to unlock it
PS2, Line 198: Nullable
I wonder if we could actually make this illegal to call prior to a connection negotiation being successful, once this refactor is complete? just a thought, no action required
PS2, Line 241: super.channelDisconnected(ctx, e); // Let the ReplayingDecoder cleanup.
i think the comment here is stale, we don't use ReplayingDecoder anymore. Does SimpleChannelUpstreamHandler have any body for this function?
PS2, Line 263: queuedMessages = Lists.newArrayList();
once we are in NEGOTIATED state, is queuedMessages ever used again? I wonder if we could null it out here?
PS2, Line 270: sendCallToWire(qm.message, qm.cb);
sendCallToWire says it's GuardedBy("lock") but here you've released it already
PS2, Line 378: // Schedule (re)connecting to the server.
: connect();
: }
in your plans for a refactor are you going to attempt to simplify this lifecycle so that an existing connection doesn't reconnect? I think that would be a noble pursuit
http://gerrit.cloudera.org:8080/#/c/7146/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java:
PS2, Line 24: queuedMessages
errant find/replace?
http://gerrit.cloudera.org:8080/#/c/7146/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
Line 134: * @return true if operations are queuedMessages, else false.
other errant find/replaces in this file
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java] separating Connection
......................................................................
Patch Set 15:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG
Commit Message:
Line 11: The updated TabletClient has been renamed into RpcHelper.
> Done
Wasn't it renamed to RpcProxy?
http://gerrit.cloudera.org:8080/#/c/7146/15/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 464: try {
Shouldn't this still be a while loop? If it throws interrupted I think it means it didn't fully wait for the process to finish.
--
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: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java-client] separating Connection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>