Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16031 )
Change subject: KUDU-1802: Avoid calls to master when using scan tokens ...................................................................... Patch Set 16: Code-Review+2 (3 comments) There is a report on an issue in LINT build. Otherwise, LGTM. http://gerrit.cloudera.org:8080/#/c/16031/14/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/16031/14/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java@66 PS14, Line 66: .addTabletServerFlag("--tserver_enforce_access_control=true"); > I am not sure what you mean. The only difference I am aware of is the clien Right: I just wanted to make sure we have the coverage for both cases where --tserver_enforce_access_control=false and --tserver_enforce_access_control=true for the new code. In particular, I was concerned that there might be some unexpected behavior if the new code in case if --tserver_enforce_access_control=false. From the code I can see it's not the case, but if looking at that as a black box, it would be nice to have the tests run against that for --tserver_enforce_access_control=false. I guess the latter is somehow covered by other tests where scan tokens are involved. http://gerrit.cloudera.org:8080/#/c/16031/14/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java@636 PS14, Line 636: .includeTabletMetadata(true) : .includeTableMetadata(false) > Sure I can add it to the size test. Thank you! http://gerrit.cloudera.org:8080/#/c/16031/15/src/kudu/client/client.proto File src/kudu/client/client.proto: http://gerrit.cloudera.org:8080/#/c/16031/15/src/kudu/client/client.proto@76 PS15, Line 76: // Cached table locations should not live longer than this timeout. : // Note: Some time will pass be > I will add a docs blurb. Yes, all these TTL and point-in-time issues are hard to be really precise with, but I think there isn't a need for precise timings, but it's better to explicitly mention that. Thank you for adding the comment! -- To view, visit http://gerrit.cloudera.org:8080/16031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c1b8392de37dd5e8b7bd8b78a21603ff8b1d1b Gerrit-Change-Number: 16031 Gerrit-PatchSet: 16 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 25 Jun 2020 03:40:23 +0000 Gerrit-HasComments: Yes
