You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2020/05/11 20:17:45 UTC

[Impala-ASF-CR] IMPALA-9382: part 1: transposed profile prototype

Tim Armstrong has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/15798 )

Change subject: IMPALA-9382: part 1: transposed profile prototype
......................................................................

IMPALA-9382: part 1: transposed profile prototype

This adds an experimental profile representation
that is denser than the traditional representation.
Counters, info strings and other information for all
instances of a fragment are merged into a single
tree. Descriptive stats (min, max, mean) are shown for
each counter, along with the values for each instance. It
can be enabled by setting --gen_experimental_profile=true.
The default behaviour is unchanged, aside from including
a few extra counters in existing profiles.

An example of the pretty-printed profile is attached
to the JIRA.

The thrift representation of the profile is extended
so that all instances of a fragment can be merged
together into a single "aggregated" fragment, with
vectors of counters.

The in-memory representation is transformed in
a similar way. The RuntimeProfile class is
restructured so that there is a common RuntimeProfileBase
class, with RuntimeProfile and AggregatedRuntimeProfile
base classes. Execution fills in counters in RuntimeProfile
for each instances, then these are aggregated together into
an AggregatedRuntimeProfile on the coordinator. This replaces
the "averaged" profile concept with an abstraction that
more clearly distinguishes what operations apply to aggregated
and unaggregated profiles.

In a future change, we could use AggregatedRuntimeProfile
for status reports so that less data needs to be sent to
the coordinator, and the coordinator needs to do less
processing.

The new profile removes the bad practice of including aggregated
stats as strings from the new profile. These stats can now be
automatically as aggregations of counters. The legacy uses of
InfoString are preserved so as to not lose information but
can be removed when we switch to the transposed profile.

Also make TotalTime and InactiveTime behave like other counters -
they are pretty-printed the same as other counters. Inactive time
is also included in the local time calculation, fixing IMPALA-2794.

TODO in later patches for IMPALA-9382:
These will need to be fixed before this can be considered production
ready.
* The JSON profile generation is not fully implemented for aggregated
  profiles.
* Not all counter times are included in aggregated profile, e.g. time
  series counters.
* The pretty-printing of the various profile counters will need to be
  improved to be more readable, e.g. grouping by host, improving
  formatting.
* The aggregated profile is only updated at the end of the query.
  We need to support live updating.
* Consider how to show local time per instance - make it a first-class
  counter?

Possible extensions:
* We could included more general summary stats for each counter, e.g.
  median, p95, etc.
* We could better highlight outliers

Testing:
* I diffed the text profile of TPC-DS Q1 to make sure there were no
  unexpected changes.

TODO
* exhaustive tests
* TSAN/ASAN

Change-Id: I0838c6a0872f57c696267ff4e92d29c08748eb7a
---
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/util/dummy-runtime-profile.h
M be/src/util/pretty-printer-test.cc
M be/src/util/pretty-printer.h
M be/src/util/runtime-profile-counters.h
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M common/thrift/RuntimeProfile.thrift
12 files changed, 1,383 insertions(+), 607 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/15798/6
-- 
To view, visit http://gerrit.cloudera.org:8080/15798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0838c6a0872f57c696267ff4e92d29c08748eb7a
Gerrit-Change-Number: 15798
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>