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 2017/01/03 23:19:42 UTC
[kudu-CR] [java-client] add missing check for open client
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] add missing check for open client
......................................................................
Patch Set 1:
> Somehow the buffers are not flushed by the time the close() method
> returns, and from a quick look there is no direct link between
> AsyncKuduClient#close and flushing outstanding sessions.
Look at the bottom of shutdown() which close() calls, you'll see that it asks to flush all the sessions.
I think the semantic that close() flushes everything is correct, and your change probably prevents some operations to complete because it needs to find a location? That does expose a challenging situation... some public APIs can be called while closing so they can't checkIfClosed(). Maybe "closed = true" should be a final step after we've flushed everything.
--
To view, visit http://gerrit.cloudera.org:8080/5190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I405d85a1295bf4e4aac348942d12efba40953a91
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
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