Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17872 )

Change subject: IMPALA-10811 RPC to submit query getting stuck for AWS NLB 
forever
......................................................................


Patch Set 29:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17872/29/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/17872/29/tests/metadata/test_ddl.py@903
PS29, Line 903: # IMPALA-10811: RPC to submit query getting stuck for AWS NLB 
forever
              : # Test HS2, Beeswax and HS2-HTTP three clients.
              : class TestAsyncDDL(TestDdlBase):
              :   @classmethod
              :   def get_workload(self):
              :     return 'functional-query'
              :
              :   @classmethod
              :   def add_test_dimensions(cls):
              :     super(TestAsyncDDL, cls).add_test_dimensions()
              :     
cls.ImpalaTestMatrix.add_dimension(create_client_protocol_dimension())
              :     
cls.ImpalaTestMatrix.add_dimension(create_exec_option_dimension(
              :         sync_ddl=[0]))
              :
              :   def test_async_ddl(self, vector, unique_database):
              :     self.run_test_case('QueryTest/async_ddl', vector, 
use_db=unique_database)
> The purpose of this test is to check out 3 different clients.
As with the JDBC test, the way I'm thinking about this test is whether this 
detects any failure cases. If we ran this test without this patch, it would 
succeed.

What defect does this test catch? What would cause this test to fail?

I consider the current test_hs2.py test better than this test, because it can 
definitely detect defects and would fail if this patch were not applied. I 
think the way forward is for that type of test to apply to all the cases that 
we care about. That may mean moving that test out of test_hs2.py to a location 
that runs for all clients. For example, that could move here and use 
ImpalaConnection's execute_async and get_state. This is implemented for beeswax 
and hs2.

https://github.com/apache/impala/blob/master/tests/common/impala_connection.py#L214
https://github.com/apache/impala/blob/master/tests/common/impala_connection.py#L224


http://gerrit.cloudera.org:8080/#/c/17872/29/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:

http://gerrit.cloudera.org:8080/#/c/17872/29/tests/metadata/test_metadata_query_statements.py@153
PS29, Line 153:   def test_async_ddl_with_JDBC(self, vector, unique_database):
              :     self.exec_with_jdbc("drop table if exists 
{0}.test_table".format(unique_database))
              :     self.exec_with_jdbc_and_compare_result(
              :         "create table {0}.test_table(a 
int)".format(unique_database),
              :         "'Table has been created.'")
              :
              :     self.exec_with_jdbc("drop table if exists 
{0}.alltypes_clone".format(unique_database))
              :     self.exec_with_jdbc_and_compare_result(
              :         "create table {0}.alltypes_clone as select * from\
              :         functional_parquet.alltypes".format(unique_database),
              :         "'Inserted 7300 row(s)'")
> It looks like the compiled java code for JDBC is based on the standard JDBC
The 21050 port is the HS2 port.
https://github.com/apache/impala/blob/master/be/src/service/impala-server.cc#L153-L154

My motivation here is that I don't want to duplicate existing tests if we can 
avoid it. From what I'm seeing, this is using HS2, and it is running a simple 
create table and a CTAS.

What type of defect can this test catch? Walk me through a time when this test 
can fail and why this test is unique.



--
To view, visit http://gerrit.cloudera.org:8080/17872
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib57e86926a233ef13d27a9ec8d9c36d33a88a44e
Gerrit-Change-Number: 17872
Gerrit-PatchSet: 29
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Oct 2021 23:14:07 +0000
Gerrit-HasComments: Yes

Reply via email to