Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14180 )
Change subject: IMPALA-8825: Add additional counters to PlanRootSink ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/14180/5/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/14180/5/be/src/service/client-request-state.h@456 PS5, Line 456: Since : /// it does not track rows read from the cache, this counter also tracks the number of : /// materialized rows, so it is used to calculate row_materialization_rate_ > nit: It only counts the number of materialized rows and it is used for deri Done http://gerrit.cloudera.org:8080/#/c/14180/5/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/14180/5/be/src/service/client-request-state.cc@117 PS5, Line 117: num_rows_fetched_cache_counter_ > nit: num_rows_fetched_from_cache_counter_ or num_result_cache_rows_counter_ Done http://gerrit.cloudera.org:8080/#/c/14180/5/tests/hs2/test_fetch_first.py File tests/hs2/test_fetch_first.py: http://gerrit.cloudera.org:8080/#/c/14180/5/tests/hs2/test_fetch_first.py@163 PS5, Line 163: def __get_runtime_profile(self, op_handle): : """Helper method to get the runtime profile from a given operation handle.""" : get_profile_req = ImpalaHiveServer2Service.TGetRuntimeProfileReq() : get_profile_req.operationHandle = op_handle : get_profile_req.sessionHandle = self.session_handle : get_profile_resp = self.hs2_client.GetRuntimeProfile(get_profile_req) : HS2TestSuite.check_response(get_profile_resp) : return get_profile_resp.profile > Feel free to defer the work but we seem to have similar functions elsewhere Yeah, it looks like the hs2_client used in test_observability is a wrapper around impyla (e.g. ImpylaHS2Connection), but the one used here is the Thrift generated ImpalaHiveServer2Service.py. This class probably pre-dated the addition of the impyla wrapper, which accounts for the difference. Agree it would be good to consolidate the two, but will defer it to another JIRA. -- To view, visit http://gerrit.cloudera.org:8080/14180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9e101e2f3e2bf8324e149c780d35825ceecc036 Gerrit-Change-Number: 14180 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 13 Sep 2019 23:41:32 +0000 Gerrit-HasComments: Yes
