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 2: (2 comments) Rebased and fixed the test in tests/custom_cluster/test_result_spooling.py http://gerrit.cloudera.org:8080/#/c/14180/1/be/src/exec/plan-root-sink.cc File be/src/exec/plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14180/1/be/src/exec/plan-root-sink.cc@48 PS1, Line 48: rows_sent_rate_ = profile_->AddDerivedCounter("RowsSentRate", TUnit::UNIT_PER_SECOND, > nit: indent seems off yeah i though that too, but according to ClangFormat that is how it should be. http://gerrit.cloudera.org:8080/#/c/14180/1/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/14180/1/be/src/service/client-request-state.h@421 PS1, Line 421: /// The number of rows materialized for this query, the counter is not reset if the : /// fetch is restarted. It does not include any rows read from the results cache, since : /// those rows are already materialized. : RuntimeProfile::Counter* rows_materialized_counter_ = nullptr; > Doesn't this track closely with num_rows_fetched_counter_ ? Would it be suf yeah, this is pretty similar to num_rows_fetched_counter_ the difference is that num_rows_fetched_counter_ counts the number of rows read from the query results cache whereas rows_materialized_counter_ does not. I've updated the commit message to make this a bit clearer. -- 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: 2 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 10 Sep 2019 22:17:11 +0000 Gerrit-HasComments: Yes
