Gokul Kolady 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 3:

(5 comments)

Will run an ASAN core test on this patch asynchronously.

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: }
            :
            : // Parses an archived thrift profile string and converts it to 
JSON.
            : static Status BuildProfileJsonFromArchive(
            :     string_view thrift_profile_archive, rapidjson::Document* 
profile_doc) {
            :   if (profile_doc == nullptr) {
            :     return Status("profile output pointer cannot be null");
            :   }
            :   thrift_profile_archive = TrimWhiteSpace(thrift_profile_archive);
            :
> We already have TrimWhiteSpace() in be/src/util/string-util.cc, can we use
Done


http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@111
PS2, Line 111:
> How about adding using std::string_view; and drop the std prefix
Done


http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@543
PS2, Line 543:
> How about adding using std::string_view; and drop the std prefix
Done


http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@542
PS2, Line 542:           Substitute("Could not parse uploaded thrift profile: 
$0", status.GetDetail()),
             :           HttpStatusCode::BadRequest);
             :       WriteJsonDocumentToStream(document, data);
             :       return;
             :     }
             :   }
             :   profile_json = &profile_doc;
             :   if
> Webserver::GetVars() can be a quite heavy operation when the post_data is l
Done


http://gerrit.cloudera.org:8080/#/c/24502/2/be/src/service/impala-http-handler.cc@558
PS2, Line 558:
> Should we have the same logic as has_query_id for "if (profile_doc.HasMembe
Done



--
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: 3
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: Fri, 24 Jul 2026 19:42:23 +0000
Gerrit-HasComments: Yes

Reply via email to