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 21: (1 comment) 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: // Transition the exec state to RUNNING for any non-CTAS DDLs. For the later, the : // state is set from PENDING to RUNNING inside ExecQueryOrDmlRequest(). : if (catalog_op_type() != TCatalogOpType::DDL : || ddl_type() != TDdlType::CREATE_TABLE_AS_SELECT) { : UpdateNonErrorExecState(ExecState::RUNNING); : } > I do not think it will work, since Exec() sets the state to RUNNING (at the Let me expand this code example: # This is pseudocode Status ExecDdlRequest() { etc etc shared code between async/sync etc etc if (async) { if (ctas) { set the state to ExecState::PENDING } else { set the state to ExecState::RUNNING } spawn async thread to do the rest return Status::OK(); } else { /// this case is not impacted and doesn't set state run synchronously, etc. } } It is better for it to be in ExecDdlRequest() prior to spawning the async thread, because that guarantees that the state is set prior to returning from Exec(). Having the ctas/non-ctas states set in adjacent code makes it very clear that they are behaving differently. That's one reason I didn't like having one thing go to PENDING in ExecDdlRequest() and then other things go to RUNNING here. -- 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: 21 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: Wed, 13 Oct 2021 23:49:25 +0000 Gerrit-HasComments: Yes
