Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9921 )
Change subject: IMPALA-6793: Fix empty metadata after statestore restarts ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9921/3/tests/custom_cluster/test_restart_services.py File tests/custom_cluster/test_restart_services.py: http://gerrit.cloudera.org:8080/#/c/9921/3/tests/custom_cluster/test_restart_services.py@26 PS3, Line 26: class TestRestart(CustomClusterTestSuite): > We can use logs, which are available, instead of metrics. This bug was found by automated tests which presumably we will not turn off. I disagree with relying on log messages for testing. I don't think this stance really needs a thorough explanation (tight coupling, not a good proxy for system state, who would expect tests to fail if log messages are changed, etc.) The harm in this test is that it will add unnecessary work for us in the future. We will find that we need to think about this test more thoroughly to make it work reliably, and then re-implement the whole thing, or worse, tweak the sleeps here. Flaky tests are a serious concern if we accumulate too many of them, so why not stop them from getting checked in? -- To view, visit http://gerrit.cloudera.org:8080/9921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I332a60e172af84b93b3544373fe363cdced5e8d0 Gerrit-Change-Number: 9921 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Wed, 04 Apr 2018 22:23:36 +0000 Gerrit-HasComments: Yes
