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