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):
> I mostly agree with you on logs. The metrics are almost as tightly coupled,
If you are happy with the test as is, I'm not going to stand in the way of 
checking it in.

Specific concerns:
1. ASAN and code coverage builds tend to have very different timings. Do we 
need to account for those? Most of our timing-based tests have two different 
times based on the build type.
2. The sleep intervals are chosen arbitrarily, but the timing depends on things 
like statestore configuration. If we change the default config of the 
statestore, this test may break.
3. If in L38 sleeping for 60s is not enough (how can we be sure?), the 
execute() might succeed based on the existing metadata cache, and then not 
cover this bug.

1. Can be addressed by changing the sleep time based on build type as done 
elsewhere
2. Can be addressed by passing specific configs to the statestore
3. Could be addressed by waiting for a change in the catalogd's and impalad's 
"statestore-subscriber.registration-id"

Probably I'm forgetting something



--
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 23:12:38 +0000
Gerrit-HasComments: Yes

Reply via email to