You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org> on 2016/11/04 16:49:51 UTC

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flushes that the client isn't waiting on
would get lost since they don't have a recipient. You could find them in the
log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
List<OperationResponses which switches us from the _errback_ to the _callback_
track. Other exceptions are sent straight back.

This patch also makes some changes for an issue that Adar saw in TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a table that
was deleted (but then the table's name was reused which made reading the logs harder).

The stray most likely came from testBatchErrorCauseSessionStuck which has
some loose ends, that this patch is fixing. It *may* make it more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
2 files changed, 93 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
OperationResponse (or a list of) which switches us from the _errback_ to the
_callback_ track. Other exceptions are still thrown.

This patch also makes some changes for an issue that Adar saw in TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a table that
was deleted (but then the table's name was reused which made reading the logs harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which isn't
checking for all the error conditions. This patch adds more checking but doesn't fix
the root cause (which is currently unknown) so it *may* make it fail more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Reviewed-on: http://gerrit.cloudera.org:8080/4949
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.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/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
6 files changed, 137 insertions(+), 70 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 3:

(1 comment)

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

PS3, Line 684: e instanceof KuduException
> nit: how fast does this work?  I.e., does it make sense to call this once a
Pretty sure it'd get JIT optimized by the JVM, but keeping a boolean would make the code clearer to read.


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4949/1//COMMIT_MSG
Commit Message:

PS1, Line 13: recipient
What does "recipient" mean in this context? If you mean a destination tserver, every flush has one (or more) of those, so you must mean something else, right?


Line 18: List<OperationResponses which switches us from the _errback_ to the _callback_
Nit: missing a '>' here.


PS1, Line 26: make it 
Nit: make it fail?


PS1, Line 26: fixing
So this patch fixes the test? Or makes it fail more often? I don't see how it'd be possible to do both.

Maybe reword this paragraph to clarify.


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

PS1, Line 664:           // We send the callback second so that the error collector's count becomes visible first,
             :           // since calling callback will wake up whoever is waiting right away and having the error
             :           // missing in the collector could be confusing.
Nit: makes sense, but can we reword for terseness? Maybe:

// Fire the callback after collecting the error so that the error is visible should the callback interrogate the error collector.


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4949/1//COMMIT_MSG
Commit Message:

PS1, Line 13: recipient
> What does "recipient" mean in this context? If you mean a destination tserv
A recipient for the flush's response. I'll reword.


Line 18: List<OperationResponses which switches us from the _errback_ to the _callback_
> Nit: missing a '>' here.
Done


PS1, Line 26: fixing
> So this patch fixes the test? Or makes it fail more often? I don't see how 
Done


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

PS1, Line 664:           // We send the callback second so that the error collector's count becomes visible first,
             :           // since calling callback will wake up whoever is waiting right away and having the error
             :           // missing in the collector could be confusing.
> Nit: makes sense, but can we reword for terseness? Maybe:
Done


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
List<OperationResponses> which switches us from the _errback_ to the _callback_
track. Other exceptions are sent straight back.

This patch also makes some changes for an issue that Adar saw in TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a table that
was deleted (but then the table's name was reused which made reading the logs harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which isn't
checking for all the error conditions. This patch adds more checking but doesn't fix
the root cause (which is currently unknown) so it *may* make it more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
2 files changed, 92 insertions(+), 33 deletions(-)


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
List<OperationResponses> which switches us from the _errback_ to the _callback_
track. Other exceptions are sent straight back.

This patch also makes some changes for an issue that Adar saw in TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a table that
was deleted (but then the table's name was reused which made reading the logs harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which isn't
checking for all the error conditions. This patch adds more checking but doesn't fix
the root cause (which is currently unknown) so it *may* make it fail more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
2 files changed, 92 insertions(+), 33 deletions(-)


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 3:

(2 comments)

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

PS3, Line 684: e instanceof KuduException
nit: how fast does this work?  I.e., does it make sense to call this once and remember the result as a boolean and then re-use the result instead of calling this again and again?


http://gerrit.cloudera.org:8080/#/c/4949/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java:

PS1, Line 578: getTabletServerError
nit: may be, buildTabletServerError() or makeTabletServerError() would be more appropriate for the name of this method?


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 3: Code-Review+1

I'll defer the +2 to Alexey; he's most familiar with the equivalent semantics in the C++ client and should compare them.

Also, perhaps you could update the release notes in this patch?

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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 4: Code-Review+2

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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

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

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

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
OperationResponse (or a list of) which switches us from the _errback_ to the
_callback_ track. Other exceptions are still thrown.

This patch also makes some changes for an issue that Adar saw in TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a table that
was deleted (but then the table's name was reused which made reading the logs harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which isn't
checking for all the error conditions. This patch adds more checking but doesn't fix
the root cause (which is currently unknown) so it *may* make it fail more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.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/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
6 files changed, 137 insertions(+), 70 deletions(-)


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
......................................................................


Patch Set 3:

> I'll defer the +2 to Alexey; he's most familiar with the equivalent
 > semantics in the C++ client and should compare them.
 > 
 > Also, perhaps you could update the release notes in this patch?

Yeah, need to do that, but I'll do it in a separate patch unless Alexey has me fix more things here.

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

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