Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/23852 )
Change subject: IMPALA-14661: Optimize admissiond memory usage by compressing exec requests ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/23852/3/be/src/rpc/sidecar-util.h File be/src/rpc/sidecar-util.h: http://gerrit.cloudera.org:8080/#/c/23852/3/be/src/rpc/sidecar-util.h@60 PS3, Line 60: // Ensure 'buf' is fully consumed into a compressed thrift in > I think calling SerializeToBuffer on the same buffer twice should be fine? Yeah, it should be fine for SerializeToBuffer() as it called resetBuffer() in https://github.com/apache/impala/blob/420e357b9586b45fbf4d08265fa57dd616989377/be/src/rpc/thrift-util.h#L105 http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/rpc/thrift-util.h File be/src/rpc/thrift-util.h: http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/rpc/thrift-util.h@285 PS5, Line 285: // Prepare compressor. : boost::scoped_ptr<Codec> compressor; : Codec::CodecInfo codec_info(THdfsCompression::LZ4); : RETURN_IF_ERROR(Codec::CreateCompressor(nullptr, false, codec_info, &compressor)); > 'compressor' and similarly 'decompressor' could be cached to avoid creating Do you mean using a thread-local compressor? In the current Impala codebase, similar code creates a new compressor each time (for example, catalog-util.cc, https://github.com/apache/impala/blob/420e357b9586b45fbf4d08265fa57dd616989377/be/src/catalog/catalog-util.cc#L345-L371). That approach seems safer, so I followed the same pattern here. If needed, maybe we can create a separate task to look into caching as a future optimization. http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/scheduling/admission-control-service.cc File be/src/scheduling/admission-control-service.cc: http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/scheduling/admission-control-service.cc@158 PS5, Line 158: if (req->has_is_compressed() && req->is_compressed()) { : admission_state->query_exec_request_compressed = : make_unique<TQueryExecRequestCompressed>(); : RESPOND_IF_ERROR(GetSidecar(req->query_exec_request_sidecar_idx(), rpc_context, : admission_s > Maybe directly check if (req->has_is_compressed()) since you're not accessi Done http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/23852/5/be/src/scheduling/admission-controller.h@568 PS5, Line 568: UpdateAdmissionRequestCompressionStats( > typo: UpdateAdmission Done -- To view, visit http://gerrit.cloudera.org:8080/23852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a676d1a806451cbf84b0a3f8a706d7c6655e12d Gerrit-Change-Number: 23852 Gerrit-PatchSet: 6 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 16 Jan 2026 00:29:03 +0000 Gerrit-HasComments: Yes
