Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 )
Change subject: IMPALA-7694: Add host resource usage metrics to profile ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/12069/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12069/7//COMMIT_MSG@15 PS7, Line 15: This mechanism adds a new time series counter class that collects all : measured values and does not re-sample them. It will re-sample values : when printing them into a string profile to a max of 64 values, but : Thrift profiles will contain the full list of values. How hard is it to factor this subset of changes out as a patch itself ? It seems easier to review that way. http://gerrit.cloudera.org:8080/#/c/12069/7/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12069/7/be/src/service/impala-server.cc@1051 PS7, Line 1051: if (rand() < trace_ratio * (RAND_MAX + 1L)) query_ctx->__set_trace_resource_usage(true); > I think rand() isn't thread safe. (And do we initialize it with srand() som Seems helpful to have a log statement that tracing is enabled. http://gerrit.cloudera.org:8080/#/c/12069/7/be/src/util/periodic-counter-updater.h File be/src/util/periodic-counter-updater.h: http://gerrit.cloudera.org:8080/#/c/12069/7/be/src/util/periodic-counter-updater.h@33 PS7, Line 33: /// Singleton utility class that updates counter values. This is used to sample some : /// metric (e.g. memory used) at regular intervals. The samples can be summarized in : /// a few ways (e.g. averaged, stored as histogram, kept as a time series data, etc). : /// This class has one thread that will wake up at a regular period and update all : /// the registered counters. : /// Typically, the counter updates should be stopped as early as possible to prevent : /// future stale samples from polluting the useful values. Should this be updated to include the role of UpdateFn() below ? -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Mon, 17 Dec 2018 23:23:59 +0000 Gerrit-HasComments: Yes
