Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/15984 )
Change subject: IMPALA-9775: Fix test failure in TestAcid.test_acid_heartbeats ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15984/1/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java File fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java: http://gerrit.cloudera.org:8080/#/c/15984/1/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java@52 PS1, Line 52: // But we have to set sleep interval not beyond 60 seconds for carrying through the : // unit test case TestAcid.test_acid_heartbeats (IMPALA-9775). > It's better to set an upper limit. Otherwise user may set an unreasonable v Normally we should use the same "hive.txn.timeout" value that HMS uses to abort transactions and locks, so sending heartbeats with timeout/3 intervals should be fine. But I don't feel too strongly about it, I'm OK with setting an upper limit. http://gerrit.cloudera.org:8080/#/c/15984/1/tests/query_test/test_acid.py File tests/query_test/test_acid.py: http://gerrit.cloudera.org:8080/#/c/15984/1/tests/query_test/test_acid.py@191 PS1, Line 191: (sleep(200000) > For current code, TestAcid.test_acid_heartbeats take 13 minutes. When incre Yeah, that's why I suggested above to remove the @pytest.mark.execute_serially annotation. This way it can run in parallel with other tests. However, it'd be possible that this test pass if other queries send heartbeats during this test's runtime (currently we don't have such tests besides this one). This might not be a problem since here we only care about Impala's capability for heartbeating. However, there's a better solution to make this test run faster. The real reason this test takes a lot of time is that it's being run with all the compressions in the test vector. There's absolutely no reason to execute this test with different compressions since we won't even use them at all. So the easiest way to reduce the execution time is to add table_format = vector.get_value('table_format') if table_format.compression_codec != 'none': pytest.skip() I think it's still worth to add these lines even if you keep the 1 minute upper limit. -- To view, visit http://gerrit.cloudera.org:8080/15984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7922797d7e3ce94a2c8948211245f4e77fdb08b7 Gerrit-Change-Number: 15984 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 26 May 2020 18:30:56 +0000 Gerrit-HasComments: Yes