Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201709191810.v8JIAnaX030951@ip-10-146-233-104.ec2.internal>
Date: Tue, 19 Sep 2017 18:10:49 +0000
From: "Sailesh Mukil (Code Review)" <gerrit@cloudera.org>
To: Tim Armstrong <tarmstrong@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Dan Hecht <dhecht@cloudera.com>
Reply-To: sailesh@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-5895=3A_clean_up_runtime_profile_lifecycle=0A?=
In-Reply-To: <gerrit.1505373167000.I45c39ac36c8e3c277213d32f5ae5f14be6b7f0df@gerrit.cloudera.org>
References: <gerrit.1505373167000.I45c39ac36c8e3c277213d32f5ae5f14be6b7f0df@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.7
archived-at: Tue, 19 Sep 2017 18:10:56 -0000

Sailesh Mukil 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_);
#ifndef NDEBUG
    lock_guard<SpinLock> l(counter_map_lock_);
    DCHECK(!has_active_periodic_counters_);
#endif

For correctness?


PS7, Line 88: void RuntimeProfile::StopPeriodicCounters() {
This doesn't stop the counters that belong to child profiles. Is it on the user to stop counters of child profiles?


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_


PS7, Line 408: sampling_counters
nit: sampling_counters_


-- 
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