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

Reply via email to