impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5487: Fix race in RuntimeProfile::toThrift()
Date Mon, 12 Jun 2017 19:05:05 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5487: Fix race in RuntimeProfile::toThrift()
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7154/1/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

PS1, Line 764: children_.size()
> The reserve() shouldn't matter for correctness, but this is wonky.
Yes, I left it there since it doesn't change correctness and I assumed in the average case
it will still be usefull. I agree that it is confusing. I could either

- Remove it
- Add a comment and explain why it is here
- Move it below where we add the children and where it may matter more

I'm in favor of removing it, the number of children seems to be small in general, but if we're
concerned about performance we may want to move it down where the children are copied.


-- 
To view, visit http://gerrit.cloudera.org:8080/7154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fad4ee2eee1f73e387c1e90a3db265b19b3a0c6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message