Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16941 )
Change subject: KUDU-3205: Fix building scan tokens when tablet not found errors occur ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java: http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java@685 PS1, Line 685: if (serverIndex != null) { > nit: for readability, consider Done http://gerrit.cloudera.org:8080/#/c/16941/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/16941/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@263 PS1, Line 263: * Get replicas of this tablet. The returned list may not be mutated. > +1 Done http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@263 PS1, Line 263: * Get replicas of this tablet. The returned list may not be mutated. > nit: do you think it's worth adding a note here expressing that the replica Done http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java: http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java@732 PS1, Line 732: KuduScanToken.KuduScanTokenBuilder tokenBuilder > nit: maybe, add includeTabletMetadata(true) and includeTableMetadata(true) Done http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java@733 PS1, Line 733: tokens > Probably, that's not exactly related to this change, bust just in case: wha The replica selection is done based off of the tabletServers list in RemoteTablet, see RemoteTablet.getClosestServerInfo. So regardless of whether the replica is missing or not the behavior is the same (it is excluded as an option). The client will refresh its location cache and re-resolve the location if needed. http://gerrit.cloudera.org:8080/#/c/16941/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java@739 PS1, Line 739: tokens.get(0).intoScanner(newClient); > Does it matter what's the role of the replica removed? Does it work as exp The token functions exactly the same as if the scan was done locally, the missing server is ignored (and can't be the leader; see RemoteTablet.removeTabletClient). Worst case scenario if a replica cannot be found the client will refresh its metadata. -- To view, visit http://gerrit.cloudera.org:8080/16941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68679dee1ad7ebca405dd6e086770f3e034e310c Gerrit-Change-Number: 16941 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 12 Jan 2021 03:46:43 +0000 Gerrit-HasComments: Yes
