Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 )
Change subject: IMPALA-7306: regression test for non-removed transient updates ...................................................................... Patch Set 3: (4 comments) Patch LGTM, some nits and clarifications. http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@773 PS3, Line 773: test_transient_entry_removal nit: test_transient_entry_removal_delayed_hbs() or something? This sounds very similar to test_topic_persistence() above. http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@778 PS3, Line 778: topic_name = "test_transient_entry_removal" Should we make sure that non-transient entries are not removed under these delayed conditions? Feel free to ignore if you think test_topic_persistence has enough coverage for it. http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@787 PS3, Line 787: with_delay nit: after_hb_failure? (or some such thing) http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@804 PS3, Line 804: + 20 why additional 20s? Doesn't the above delayed HB cause it to be unsubscribed before WAIT_FOR_FAILURE_TIMEOUT? -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 20 Sep 2018 19:12:32 +0000 Gerrit-HasComments: Yes
