Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 )
Change subject: IMPALA-6812: Fix flaky Kudu scan tests ...................................................................... Patch Set 1: > It looks like there's some history here, but can we make > READ_AT_SNAPSHOT the default? It would be best to test the default > mode as much as possible. I'm sympathetic to the problem that people may still encounter these test failures if they run the tests through a means other than run-all-tests.sh I don't think though that we should be choosing default values based on what's convenient for testing, they should be based on what's best for users. If READ_AT_SNAPSHOT is the better choice for general usage, maybe we can change it, but I'm not sure that anything has changed about the trade-off between READ_AT_SNAPSHOT and READ_LATEST since the default was chosen that would affect the calculation. I'm also concerned that changing this default is significant enough to be considered breaking. There are some other options to address this flakiness: - Modify all Kudu tests to retry scans if they don't return the expected results. This is of course a large change, still a little error prone if people write future tests that don't follow the convention, and creates even more reliance on timeouts/retries in our tests, which is a common source of continuing flakiness. - Introduce a mocking framework and run these tests against a mock Kudu. Part of the underlying issue here (and the same with the Kudu test timeout patch I also have out right now) is that we're testing Kudu's functionality in all of these tests. I think there's a strong argument to be made that we would be better off letting the Kudu team do the majority of the testing of Kudu functionality, and to focus our tests more narrowly at Impala-specific functionality (though of course we would still want at least a few real integration tests). Even if we think one of the above options is better, I would still suggest we get this change in to reduce test flakiness asap, since they'll both require a lot more work. I'm adding Todd to the review to see if he has an opinion on what the best default value is. -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 25 May 2018 21:24:43 +0000 Gerrit-HasComments: No
