Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14242 )
Change subject: IMPALA-8884: track storage read statistics per queue ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/14242/4/be/src/runtime/io/disk-io-mgr.cc File be/src/runtime/io/disk-io-mgr.cc: http://gerrit.cloudera.org:8080/#/c/14242/4/be/src/runtime/io/disk-io-mgr.cc@148 PS4, Line 148: DEFINE_bool(cache_s3_file_handles, true, : "Enable the file handle cache for " : "S3 files."); > Nit: There are some edge cases the code formatter doesn't handle. This one Done - reverted to old formatting. http://gerrit.cloudera.org:8080/#/c/14242/4/be/src/runtime/io/disk-io-mgr.cc@288 PS4, Line 288: FindMetricForTesting > This is a pretty uncommon case where tests register metrics twice, so it pr I added an is_test() check beforehand so that this is only called when running in a test environment. http://gerrit.cloudera.org:8080/#/c/14242/4/be/src/runtime/io/disk-io-mgr.cc@294 PS4, Line 294: HistogramMetric* read_latency = : ImpaladMetrics::IO_MGR_METRICS->FindMetricForTesting<HistogramMetric>( : Substitute(READ_LATENCY_METRIC_KEY_TEMPLATE, i_string)); : disk_queues_[i]->set_read_latency(read_latency != nullptr ? read_latency : : ImpaladMetrics::IO_MGR_METRICS->RegisterMetric(new HistogramMetric( : MetricDefs::Get(READ_LATENCY_METRIC_KEY_TEMPLATE, i_string), : ONE_HOUR_IN_NS, 3))); > For these cases, we expect read_latency to be null unless this is a backend I changed it around a bit so that the FindMetricForTesting() code only runs as part of tests. -- To view, visit http://gerrit.cloudera.org:8080/14242 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8233ed02b418f22f1d0c031e378288357796f4b4 Gerrit-Change-Number: 14242 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 08 Oct 2019 00:53:27 +0000 Gerrit-HasComments: Yes
