You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/06/24 01:50:47 UTC

[kudu-CR] [java-client] refactor AsyncKuduSession

Hello Jean-Daniel Cryans, Adar Dembo,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/3477

to review the following change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
8 files changed, 587 insertions(+), 638 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3477/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

PS2, Line 396: Must not be concurrently accessed
I'm still thrown off by this comment. Maybe:
* @param buffer the buffer to flush, must not be modified once passed to this method


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 6: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

PS1, Line 1400: locateTablet
How does this related to the other locateTablet() method? It seems to be doing something different, so at least it shouldn't be called the same?


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

PS1, Line 396: Must not be concurrently accessed
It already is it seems though, the client can call it with a flusher task fires.


Line 705:     private List<BufferedOperation> operations = new ArrayList<>();
final


PS1, Line 725: it's
its


PS1, Line 725: (
not closing the parenthesis in the comment


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/RowError.java
File java/kudu-client/src/main/java/org/kududb/client/RowError.java:

Line 87:         ", tablet=" + (operation.getTablet() == null ? null :
Just put getTablet there directly, RemoteTablet has toString.


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
File java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java:

Line 54:    * Workaround for {@link Deferred#addBoth}'s failure to use generics correctly.
Can you be more clear regarding what this does and why someone should use it?


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3477

to look at the new patch set (#5).

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
8 files changed, 590 insertions(+), 638 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/5
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3477

to look at the new patch set (#4).

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
8 files changed, 590 insertions(+), 638 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3477

to look at the new patch set (#6).

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
9 files changed, 592 insertions(+), 638 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/6
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/2121/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/2096/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 409:     Deferred<List<BatchResponse>> batchResponses = new Deferred<>();
> Nit: not really a fan of sometimes using Deferred.fromResult() and other ti
Here is not a case with a result though, it's just creating a Deferred.


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/2092/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1976/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1410:       locatedTablets = locateTable(table, partitionKey,
> So this is how we obtain the very next value following partitionKey, right?
yes.  IncrementEncodedKey is more complicated because it has to produce a valid key that could be decoded.  This key doesn't need to be decoded, so it doesn't have to be as complicated.  For instance, if the key was just a single int32 column this would result in 5 bytes, but it's only used for comparing so it doesn't matter.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 123:    * application is {@link #apply}ing operations to one buffer (the {@code activeBuffer}), the
> Nit: {@link #apply} or {@code apply}? I see both here, not sure which is mo
I try and use @link for the first reference and @code thereafter.


Line 283:     return flush();
> Why flush() if the session is already closed?
flush() is already a no-op if the active buffer is null.


PS3, Line 425: private static final ConvertBatchToListOfResponsesCB INSTANCE =
             :         new ConvertBatchToListOfResponsesCB();
> Is this for correctness or performance? Seems like the implementation would
It's just to save allocation overhead since this is a static class with no fields.  Removing the caching wouldn't change the logic in #call.


Line 563:         doFlush(fullBuffer);
> What happens if doFlush() throws but we're executing it because we throw a 
Not sure, but I don't think doFlush can throw.  In general, methods that return Deferred shouldn't throw (the notable exception to this is PleaseThrottleException).


Line 704:   private final class Buffer {
> Shouldn't Buffer be declared static? Or did you intend for it to be an inne
Flusher is non-static, so transitively Buffer must be non-static.  I use final because it's a signal that you don't have to worry about OO shenanigans.  Otherwise you have to search the file to make sure.


Line 748:       flushNotification = new Deferred<>();
> Nit: use fromResult(null) here to be consistent with the inline field const
fromResult(null) creates a completed deferred, whereas new Deferred() creates an incomplete deferred.  When the buffer is created it's not flushing, so the flush notification should be complete. When it's reset it's being made into the active buffer, so the deferred is changed to an incomplete deferred.


Line 756:                         .add("flusherTask", flusherTask)
> This will just be an object address, right? Useful?
Yes, just to distinguish it from null.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Batch.java
File java/kudu-client/src/main/java/org/kududb/client/Batch.java:

Line 46:   final List<Operation> operations = new ArrayList<>(125);
> What's magical about 1250?
nothing, and in fact it should probably be more like 125 (default buffer size / 8).  8 is similarly arbitrary, but seems like a reasonable number of tablets for a table.  There's really no way to predict how big this should be.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Operation.java
File java/kudu-client/src/main/java/org/kududb/client/Operation.java:

Line 203:   static Tserver.WriteRequestPB.Builder createAndFillWriteRequestPB(List<Operation> operations) {
> What motivated the changes here from variadic args to lists? Isn't the form
In the callsite where we had a variable (large) number of operations (Batch), we were copying into an array from an array list.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
File java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java:

Line 22: import org.kududb.annotations.InterfaceAudience;
> Unused?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3477/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

PS2, Line 396: Must not be concurrently accessed
> I'm still thrown off by this comment. Maybe:
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

PS1, Line 1400: locateTablet
> How does this related to the other locateTablet() method? It seems to be do
Done


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

PS1, Line 396: Must not be concurrently accessed
> It already is it seems though, the client can call it with a flusher task f
The flusher task and the client task both lock the monitor and set the active buffer to null before calling this, so I don't think it's possible.


Line 705:     private List<BufferedOperation> operations = new ArrayList<>();
> final
Done


PS1, Line 725: (
> not closing the parenthesis in the comment
Done


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/client/RowError.java
File java/kudu-client/src/main/java/org/kududb/client/RowError.java:

Line 87:         ", tablet=" + (operation.getTablet() == null ? null :
> Just put getTablet there directly, RemoteTablet has toString.
Done


http://gerrit.cloudera.org:8080/#/c/3477/1/java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
File java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java:

Line 54:    * Workaround for {@link Deferred#addBoth}'s failure to use generics correctly.
> Can you be more clear regarding what this does and why someone should use i
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 563:         doFlush(fullBuffer);
> Not sure, but I don't think doFlush can throw.  In general, methods that re
I think doFlush() may throw (unchecked excpetions) if the buffer passed to it was empty, in which case it'd callback() on the global flushNotification deferred, which would potentially run user code that would throw.

It doesn't seem like that could happen from here (where fullBuffer is, well, full) but I wonder what that means for other doFlush() callers.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Batch.java
File java/kudu-client/src/main/java/org/kududb/client/Batch.java:

Line 46:   final List<Operation> operations = new ArrayList<>(125);
> nothing, and in fact it should probably be more like 125 (default buffer si
So why bother predicting it at all?

Anyway, if you do retain the preallocation, please add a comment explaining why 125 was chosen.


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Reviewed-on: http://gerrit.cloudera.org:8080/3477
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
9 files changed, 592 insertions(+), 638 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2038/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3477

to look at the new patch set (#3).

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
8 files changed, 593 insertions(+), 639 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3477

to look at the new patch set (#2).

Change subject: [java-client] refactor AsyncKuduSession
......................................................................

[java-client] refactor AsyncKuduSession

This commit refactors how AsyncKuduSession handles buffering internally. Instead
of using a buffer per tablet and a separate list of operations in tablet
location lookup, we buffer all operations into a single large buffer, and then
dispatch into per-tablet batches during flush.

This result is simplified logic in AsyncKuduSession. AsyncKuduSession#apply is
no longer reentrant, and lookups no longer need to be tracked explicitly.

This patch was tested against YCSB to make sure that it does not result in
performance regressions.  With an increased buffer size of 5000 to match the 0.9
client's buffer size, the performance is largely the same. With this refactor
the effective batch size is much smaller since mutationBufferSize now refers to
the one buffer for all tablets instead of to a per-tablet buffer.

Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/Bytes.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
M java/kudu-client/src/main/java/org/kududb/client/RowError.java
M java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java
M java/kudu-client/src/test/java/org/kududb/client/TestOperation.java
8 files changed, 593 insertions(+), 639 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3477/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 7:

Should we change the default buffer size up by an order of magnitude so that users don't experience this as a regression? We should also release note the changed client behavior.

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
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] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 563:         doFlush(fullBuffer);
> I think doFlush() may throw (unchecked excpetions) if the buffer passed to 
I've changed doFlush to be a no-op when the buffer is empty just to make sure that a PTE callback can not be executed.


http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Batch.java
File java/kudu-client/src/main/java/org/kududb/client/Batch.java:

Line 34: import org.kududb.tserver.Tserver;
> Odd that this comes after Tserver.TabletServerErrorPB.
Done


Line 46:   final List<Operation> operations = new ArrayList<>(1250);
> So why bother predicting it at all?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1984/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3477/5/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 405:       // no-op.
> Can you make sure we have a test for this?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3477/5/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 405:       // no-op.
Can you make sure we have a test for this?


-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/2118/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] refactor AsyncKuduSession

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
......................................................................


Patch Set 4: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/2094/

-- 
To view, visit http://gerrit.cloudera.org:8080/3477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No