Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24502 )
Change subject: IMPALA-15107: Support Direct Thrift Upload for AI Profile Analyzer ...................................................................... Patch Set 2: (4 comments) Can we run an ASAN core test on this patch to see if there are any memory related issues? http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@98 PS2, Line 98: : static std::string_view TrimAsciiWhitespace(std::string_view input) { : while (!input.empty() && isspace(static_cast<unsigned char>(input.front()))) { : input.remove_prefix(1); : } : while (!input.empty() && isspace(static_cast<unsigned char>(input.back()))) { : input.remove_suffix(1); : } : return input; : } We already have TrimWhiteSpace() in be/src/util/string-util.cc, can we use that one? http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@111 PS2, Line 111: std::string_view How about adding using std::string_view; and drop the std prefix http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@543 PS2, Line 543: std:: How about adding using std::string_view; and drop the std prefix http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@542 PS2, Line 542: Webserver::ArgumentMap post_args = Webserver::GetVars(req.post_data); : std::string_view thrift_profile_archive; : const auto thrift_profile_arg = post_args.find("thrift_profile"); : if (thrift_profile_arg != post_args.end()) { : thrift_profile_archive = thrift_profile_arg->second; : } else { : thrift_profile_archive = req.post_data; : } Webserver::GetVars() can be a quite heavy operation when the post_data is large, and maybe in most of cases, we don't have the thrift_profile. How about we check the raw data for thrift_profile=, then if yes, do Webserver::GetVars(), like: string_view thrift_profile_archive = req.post_data; if (req.post_data.find("thrift_profile=") != std::string::npos) { Webserver::ArgumentMap post_args = Webserver::GetVars(req.post_data); const auto thrift_profile_arg = post_args.find("thrift_profile"); if (thrift_profile_arg != post_args.end()) { thrift_profile_archive = thrift_profile_arg->second; } } -- To view, visit http://gerrit.cloudera.org:8080/24502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff05c3cde7b34a56e31f020611cc33ff4e02aa32 Gerrit-Change-Number: 24502 Gerrit-PatchSet: 2 Gerrit-Owner: Gokul Kolady <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Gokul Kolady <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 23 Jul 2026 18:34:54 +0000 Gerrit-HasComments: Yes
