Bharath Vissapragada has posted comments on this change. ( )

Change subject: IMPALA-8661 : Add randomized tests to stress 

Patch Set 5:


Great work, exhaustive coverage. I skimmed through most parts of the patch. 
seems reasonable to me.  I think it is difficult to review the entire patch in 
a single go. We should probably commit it and keep fixing any issues 
incrementally. Since most of the patch is isolated to test framework, any bugs 
(like connection leaks, if any) shouldn't affect the product itself.
Commit Message:
PS5, Line 35: 1. Ran the test with defaults. It generates about 2100 events
Should we consider lower defaults for core tests? 15mins seems like a 
considerable increase in test execution time.
File fe/src/main/java/org/apache/impala/catalog/events/
PS5, Line 299:       infoLog("Notification event received");
nit: isn't this too chatty for info?
PS5, Line 46: refresh
nit: grammar
PS5, Line 83: else
do we want to override numClients in 3.x? in which case it should probably be 
something like

if (version < 3 && numClients != null)...
PS5, Line 138: import org.mockito.Mockito;
File fe/src/test/java/org/apache/impala/testutil/
PS5, Line 28: import java.util.concurrent.BlockingQueue;
nit: duplicate.
PS5, Line 36: public class HiveJdbcClientPool implements Closeable {
class comment.
PS5, Line 71:   if (conn_ == null) {
            :         throw new RuntimeException("Connection not initialized.");
            :       } else if (conn_.isClosed()) {
            :         throw new RuntimeException("Connection not open.");
            :       }
Make them preconditions?
PS5, Line 94:     public boolean executeSql(String sql) throws SQLException {
method doc.
PS5, Line 146: closedCount > 0
!freeClients_.isEmpty() ?
File fe/src/test/java/org/apache/impala/testutil/
PS5, Line 42: import org.apache.commons.lang3.RandomStringUtils;
File fe/src/test/java/org/apache/impala/util/
PS5, Line 117: for
nit: remove
PS5, Line 345: exists

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c85b83efd4f56b5ae0e8d1dc6a2ee2feb6721ce
Gerrit-Change-Number: 13932
Gerrit-PatchSet: 5
Gerrit-Owner: Vihang Karajgaonkar <>
Gerrit-Reviewer: Anurag Mantripragada <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Impala Public Jenkins <>
Gerrit-Reviewer: Quanlong Huang <>
Gerrit-Reviewer: Vihang Karajgaonkar <>
Gerrit-Comment-Date: Tue, 30 Jul 2019 07:32:53 +0000
Gerrit-HasComments: Yes

Reply via email to