Tim Armstrong 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) 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 v I just added _race as a suffix to be more concise - I think that captures that it testing for a particular race. 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 Done 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) Done 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 unsubscribe Actually we can set it to zero, since the first update occurs after the failure. Updated and commented to explain. Had to fix wait_for_failure to handle a 0 timeout correctly too. -- 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: Mon, 24 Sep 2018 17:57:53 +0000 Gerrit-HasComments: Yes
