impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5895: clean up runtime profile lifecycle
Date Tue, 19 Sep 2017 20:08:22 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5895: clean up runtime profile lifecycle
......................................................................


Patch Set 7:

(4 comments)

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

PS7, Line 85: DCHECK(!has_active_periodic_counters_);
> We shouldn't take locks only under debug builds since they have side effect
Also the destructor can't safely run concurrently with other method calls regardless.


PS7, Line 88: void RuntimeProfile::StopPeriodicCounters() {
> This doesn't stop the counters that belong to child profiles. Is it on the 
Yeah, my thinking was that whatever added the counters to the profile should also explicitly
stop them. Updated the method comment since it wasn't very clear.


http://gerrit.cloudera.org:8080/#/c/8069/7/be/src/util/runtime-profile.h
File be/src/util/runtime-profile.h:

PS7, Line 407: rate_counters
> nit: rate_counters_
Done


PS7, Line 408: sampling_counters
> nit: sampling_counters_
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I45c39ac36c8e3c277213d32f5ae5f14be6b7f0df
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message