Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24162 )
Change subject: IMPALA-14872: Perform work outside DoFuncForAllEntries ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/24162/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/24162/1/be/src/service/impala-server.cc@3066 PS1, Line 3066: DoFuncForAllEntries I saw in callstacks other uses of DoFuncForAllEntries from Impala's webui (e.g. queries page). It would be nice to reduce locking in those cases too. A problematic one is ImpalaHttpHandler::QueryStateHandler - it creates QueryStateRecord during DoFuncForAllEntries, and QueryStateRecord::Init() calls coord->ComputeQueryResourceUtilization(), which again takes the lock for each BackendState http://gerrit.cloudera.org:8080/#/c/24162/1/be/src/service/impala-server.cc@3081 PS1, Line 3081: if (query_driver->finalized()) continue; : ClientRequestState* request_state = query_driver->GetActiveClientRequestState(); Can it be a problem if finalization happens just after query_driver->finalized() from another thread? It looks cleaner to me to add a function do QueryDriver that takes care of locking, e.g GetQueryIdIfBackandLagAbove(max_lag_ms); http://gerrit.cloudera.org:8080/#/c/24162/1/be/src/service/impala-server.cc@3082 PS1, Line 3082: : Coordinator* coord = request_state->GetCoordinator(); I am not very familiar with query retry, but AFAIK query_driver->GetActiveClientRequestState() may return a different result here then in the lambda above if the query is retried. This may be lead to issues, e.g. if this new retry has no coordinator yet, so the coord's nullness should be checked again. Could be also avoided by collecting ClientRequestStates in the lambda above. -- To view, visit http://gerrit.cloudera.org:8080/24162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49f92bf4fe1b773ee8643422091b7252ddd81685 Gerrit-Change-Number: 24162 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Thu, 02 Apr 2026 11:06:22 +0000 Gerrit-HasComments: Yes
