Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/19428 )
Change subject: IMPALA-11850 Adds HTTP tracing headers when using the hs2-http protocol. ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/19428/7/be/src/transport/THttpServer.cpp File be/src/transport/THttpServer.cpp: http://gerrit.cloudera.org:8080/#/c/19428/7/be/src/transport/THttpServer.cpp@180 PS7, Line 180: } else if (THRIFT_strncasecmp(header, HEADER_IMPALA_SESSION_ID.c_str(), sz) == 0) { > probably best to use StripWhiteSpace instead of boost::trim. good catch, fixed http://gerrit.cloudera.org:8080/#/c/19428/7/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/19428/7/shell/impala_client.py@434 PS7, Line 434: transport.addCustomHeaderFunc(self.get_custom_http_headers) > Unless I'm missing something, the headers aren't getting updated for subseq This line passes in the get_custom_http_headers function itself, not the returned value from that function. Then, in the ImpalaHttpClient, it calls that function to get the current request id, session id, and query id. This solution is necessary because of how the object structure is set up. The request id, session id, and query id are all managed by the ImpalaHS2Client but the ImpalaHS2Client does not know what transport it's using. It just has to be a subclass of TTransportBase. Since TTransportBase comes from Thrift, its definition cannot be modified to pass in a request context object. To complicate matters even more, when the hs2-http protocol is used, impala_client.py wraps the ImpalaHttpClient object inside a TBufferedTransport object from Thrift (see https://github.com/apache/impala/blob/cda41146902dfebd101b5906c031f562e3094889/shell/impala_client.py#L434). Consequently, getting to the actual ImpalaHttpClient is not possible because its held in a private instance variable of the TBufferedTransport object and we cannot modify the structe of TBufferedTransport because it comes from Thrift. Thus, this solution which calls back to the ImpalaHS2Client to get the request id, session id, and query id. -- To view, visit http://gerrit.cloudera.org:8080/19428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7857eb5ec03eba32e06ec8d4133480f2e958ad2f Gerrit-Change-Number: 19428 Gerrit-PatchSet: 8 Gerrit-Owner: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Comment-Date: Thu, 02 Feb 2023 22:30:57 +0000 Gerrit-HasComments: Yes
