Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23343 )
Change subject: IMPALA-14348: Fix TSAN issue after IMPALA-14327 ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/23343/1/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/23343/1/be/src/service/impala-hs2-server.cc@645 PS1, Line 645: query_handle->returns_result_set() > Can we confirm whether returns_result_set_ is immutable after setup. If it returns_result_set() is accessor to result_metadata_. There are many callsites in client-request-state.cc and client-request-state.h where result_metadata_ is assigned a new value. There is already example in ImpalaServer::GetResultSetMetadata() where accessing ClientRequestState::result_metadata() is preceeded by obtaining ClientRequestState::lock_ first. http://gerrit.cloudera.org:8080/#/c/23343/1/be/src/service/impala-hs2-server.cc@647 PS1, Line 647: query_handle->returns_result_set()); > Can result_metadata_ actually get modified in parallel? It seems like an er The TSAN error spot race between this read and write at https://github.com/apache/impala/blob/2fb56afb5e645a0c4e2b9bc3ab55e52cbee9fac0/be/src/service/client-request-state.cc#L832 The write path hold lock_. But this read path was not. http://gerrit.cloudera.org:8080/#/c/23343/1/be/src/service/impala-hs2-server.cc@673 PS1, Line 673: *result_set_md, nullptr, query_handle->query_options().stringify_map > can you confirm if it would try to grab lock_ again internally with a comme You're right. Excluded SetResultCache from the lock scope. -- To view, visit http://gerrit.cloudera.org:8080/23343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I41fc25cea5b4ef7b4b9daac54b8665fa76ceb1cd Gerrit-Change-Number: 23343 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 25 Aug 2025 14:34:38 +0000 Gerrit-HasComments: Yes
