You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Tony Foerster (Code Review)" <ge...@cloudera.org> on 2018/07/31 15:41:01 UTC
[kudu-CR] KUDU-2348 return random replica from RemoteTablet
Tony Foerster has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11088
Change subject: KUDU-2348 return random replica from RemoteTablet
......................................................................
KUDU-2348 return random replica from RemoteTablet
RemoteTablet is returning a non-random replica based on the hash of the
tablet servers when no local replica exists and a leader isn't requested.
This commit changes uses a random seed to return a random replica per
RemoteTablet instance. RemoteTablet instances are kept in the
TabletLocationsCache for 1 hour by default. When the cache is invalidated
and a new RemoteReplica is created, a new random replica will be returned.
It's possible to return a completely random replica, but this would make
implementing features like a scanner keepalive more difficult.
Change-Id: I6d0b0f92dafc56786c2e915f88daf83afedc8f90
---
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
2 files changed, 41 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11088/1
--
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: newchange
Gerrit-Change-Id: I6d0b0f92dafc56786c2e915f88daf83afedc8f90
Gerrit-Change-Number: 11088
Gerrit-PatchSet: 1
Gerrit-Owner: Tony Foerster <an...@gmail.com>
[kudu-CR] KUDU-2348 return random replica from RemoteTablet
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <an...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 31 Jul 2018 15:52:08 +0000
Gerrit-HasComments: Yes