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 31: (3 comments) Ok, this is getting close. I have a couple small code comments. I wrote some additional tests here: http://gerrit.cloudera.org:8080/17959 You are welcome to incorporate them into your change, or I can merge them separately. http://gerrit.cloudera.org:8080/#/c/17872/31/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/17872/31/be/src/service/client-request-state.cc@695 PS31, Line 695: Async Nit: What do you think about dropping Async from this name? http://gerrit.cloudera.org:8080/#/c/17872/31/be/src/service/client-request-state.cc@702 PS31, Line 702: // Indirectly check if running in thread async_exec_thread_. : if (!exec_dml_async) { When I was reading this code, I got confused by the variable name. The code inside this if is for when we are doing async DML. Should this variable be called "exec_dml_sync"? http://gerrit.cloudera.org:8080/#/c/17872/31/be/src/service/client-request-state.cc@706 PS31, Line 706: DebugActionNoFail( : exec_request_->query_options, "CRS_DELAY_BEFORE_CATALOG_OP_EXEC"); When adding a delay, it usually lines up with some piece of code that can take a long time. This pause is standing on its own a bit away from the actual catalog op that could take a long time, but Impala won't actually pause in this location. Can we put it right next to the catalog op? -- 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: 31 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, 21 Oct 2021 02:47:36 +0000 Gerrit-HasComments: Yes
