Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11088 )
Change subject: KUDU-2348 return random replica from RemoteTablet ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/11088/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11088/1//COMMIT_MSG@16 PS1, Line 16: It's possible to return a completely random replica, but this would make : implementing features like a scanner keepalive more difficult. well, I dont think scanner keepalive should rely on replica selection at all, exactly for this reason. It needs to be pinned to whichever replica _was_ selected, even if the selection criteria dynamically changes for the next scan. I think the other downside of random selection for each access is cache warming -- we probably want to be sticky to a particular replica from a single client so that we get good read cache locality. http://gerrit.cloudera.org:8080/#/c/11088/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java: http://gerrit.cloudera.org:8080/#/c/11088/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@22 PS1, Line 22: import java.util.*; we usually discourage wildcard imports http://gerrit.cloudera.org:8080/#/c/11088/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@31 PS1, Line 31: import com.sun.security.ntlm.Server; ? http://gerrit.cloudera.org:8080/#/c/11088/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@65 PS1, Line 65: single RemoteTablet instance. what's the lifetime of RemoteTablet? maybe better to just use a random seed when the client is created? Any idea what HDFS does, btw? http://gerrit.cloudera.org:8080/#/c/11088/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@191 PS1, Line 191: new ArrayList<>(tabletServerValues).get(randomIndex) this seems expensive to allocate and copy here. Instead could we embed the picking into the above loop? -- To view, visit http://gerrit.cloudera.org:8080/11088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d0b0f92dafc56786c2e915f88daf83afedc8f90 Gerrit-Change-Number: 11088 Gerrit-PatchSet: 1 Gerrit-Owner: Tony Foerster <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 31 Jul 2018 15:52:08 +0000 Gerrit-HasComments: Yes
