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/02/20 00:16:34 UTC

[Impala-ASF-CR] IMPALA-9381: lazy conversion of runtime profile

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

Change subject: IMPALA-9381: lazy conversion of runtime profile
......................................................................

IMPALA-9381: lazy conversion of runtime profile

Converting the runtime profile to JSON and text representations
at the end of the query used significant CPU and time. These
representations will commonly never be accessed, because
they need to be explicitly requested by a client via the
HTTP debug interface or via a thrift profile request.
So it is a waste of resources to eagerly convert them, and
in particular it is a bad idea to do so on the critical path
of a query.

This commit switches to generating alternative profile
representations on-demand. Only the compressed thrift version
of the profile is stored in QueryStateRecord. This is the
most compact representation of the profile and and it is
relatively convenient to convert into other formats.

Also use a move() when constructing QueryStateRecord to avoid
copying the profile unnecessarily.

Fix a couple of potential use-after-free issues where Json
objects generated by RuntimeProfile::ToJson() could reference
strings owned by the object pool. These were detected by
running an ASAN build, because after this change, the temporary
object pool used to hold the deserialized profile was freed before
the JSON tree was returned.

The "kind" field of counters is removed from the JSON profile.
This couldn't be round-tripped correctly through thrift, and
probably isn't necessary. It also helps slim down the profiles.

Also make sure to preserve the "indent" field when round-tripping
to thrift.

Testing:
Ran core tests.

Diffed JSON and text profiles download from web UI from before and
after to make sure there were no unexpected changes as a result
of the round-trip via thrift.

Change-Id: Ic2f5133cc146adc3b044cf4b64aae0a9688449fa
---
M be/src/service/impala-http-handler.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.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
7 files changed, 143 insertions(+), 138 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/15236/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15236
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2f5133cc146adc3b044cf4b64aae0a9688449fa
Gerrit-Change-Number: 15236
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>