Adar Dembo has posted comments on this change. Change subject: [java client] Extract ip2client out of AsyncKuduClient ......................................................................
Patch Set 1: (3 comments) I looked at the ConnectionCache API and members, but only skimmed the method implementations because I assumed that was just code motion. Was there any implementation change worth looking at? http://gerrit.cloudera.org:8080/#/c/4717/1/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: Line 91: this.kuduClient = client; Isn't this a reference cycle between the AsyncKuduClient and ConnectionCache, which would prevent the two from being GC'ed? I think it would be cleaner if you passed in the facilities that ConnectionCache needs (e.g. the timer, the channel factory, etc.). Then you wouldn't need getters for those on AsyncKuduClient either. PS1, Line 167: AsyncKuduClient Odd to reference this class here, no? Line 279: static String getIP(final String host) { Since this is static, I think it could remain in AsyncKuduClient, to keep the ConnectionCache API slimmer. -- To view, visit http://gerrit.cloudera.org:8080/4717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
