Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14180 )
Change subject: IMPALA-8825: Add additional counters to PlanRootSink ...................................................................... Patch Set 5: Code-Review+2 (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 deriving the row_materialization_rate_. 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_ or something simpler 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 (https://github.com/apache/impala/blob/master/tests/query_test/test_observability.py#L77) in other tests so it'd be nice to consolidate them in the future. -- 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 22:29:49 +0000 Gerrit-HasComments: Yes
