Qifan Chen 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 25:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17872/21/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/17872/21/be/src/service/client-request-state.cc@713
PS21, Line 713:   // If this is a CTAS request, there will usually be more work 
to do
              :   // after executing the CREATE TABLE statement (the INSERT 
portion of the operation).
              :   // The exception is if the user specified IF NOT EXISTS and 
the table already
              :   // existed, in which case we do not execute the INSERT.
              :   if (catalog_op_type() == TCatalogOpType::DDL &&
              :
> Let me expand this code example:
The difference with the state in main thread for CTAS before overwritten by the 
child thread is as follows.

1. Previous code: INITIALIZED
2. Your proposal: PENDING

In either version, the state is set prior to returning from Exec().

For non-CTAS DDLs, the state in main thread before overwritten by the child 
thread is as follows.

1. Previous code: INITIALIZED
2. Your proposal: RUNNING

It seems we disagree on when to set the state and by which thread. My thinking 
is that it should be the responsibility of the thread who does the state change 
(i.e., ExecDdlRequestImplAsync() when running in async_exec_thread_). In the 
current code, the transition is set at the beginning and a better place is 
after the delay via CRS_DELAY_BEFORE_CATALOG_OP_EXEC.

Please let me know. I probably will not be able to work on it until next week.


http://gerrit.cloudera.org:8080/#/c/17872/25/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/17872/25/be/src/service/client-request-state.cc@773
PS25, Line 773:       unique_lock<mutex> unique_lock(lock_);
> You don't need this lock, so let's remove it.
Done



--
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: 25
Gerrit-Owner: Qifan Chen <[email protected]>
Gerrit-Reviewer: Amogh Margoor <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Comment-Date: Thu, 14 Oct 2021 02:00:07 +0000
Gerrit-HasComments: Yes

Reply via email to