Return-Path: <reviews-return-18569-archive-asf-public=cust-asf.ponee.io@impala.incubator.apache.org>
X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io
Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io
Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183])
	by cust-asf2.ponee.io (Postfix) with ESMTP id 8C25E20049C
	for <archive-asf-public-internal@cust-asf2.ponee.io>; Fri, 11 Aug 2017 20:13:58 +0200 (CEST)
Received: by cust-asf.ponee.io (Postfix)
	id 8A89916C3F6; Fri, 11 Aug 2017 18:13:58 +0000 (UTC)
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by cust-asf.ponee.io (Postfix) with SMTP id D06D416C3D4
	for <archive-asf-public@cust-asf.ponee.io>; Fri, 11 Aug 2017 20:13:57 +0200 (CEST)
Received: (qmail 11016 invoked by uid 500); 11 Aug 2017 18:13:57 -0000
Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:reviews-help@impala.incubator.apache.org>
List-Unsubscribe: <mailto:reviews-unsubscribe@impala.incubator.apache.org>
List-Post: <mailto:reviews@impala.incubator.apache.org>
List-Id: <reviews.impala.incubator.apache.org>
Delivered-To: mailing list reviews@impala.incubator.apache.org
Received: (qmail 11004 invoked by uid 99); 11 Aug 2017 18:13:56 -0000
Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 11 Aug 2017 18:13:56 +0000
Received: from localhost (localhost [127.0.0.1])
	by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 452B8C00E2
	for <reviews@impala.apache.org>; Fri, 11 Aug 2017 18:13:56 +0000 (UTC)
X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org
X-Spam-Flag: NO
X-Spam-Score: 0.362
X-Spam-Level: 
X-Spam-Status: No, score=0.362 tagged_above=-999 required=6.31
	tests=[RDNS_DYNAMIC=0.363, SPF_PASS=-0.001] autolearn=disabled
Received: from mx1-lw-eu.apache.org ([10.40.0.8])
	by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024)
	with ESMTP id ulQqubPH8uT2 for <reviews@impala.apache.org>;
	Fri, 11 Aug 2017 18:13:54 +0000 (UTC)
Received: from ip-10-146-233-104.ec2.internal (ec2-75-101-130-251.compute-1.amazonaws.com [75.101.130.251])
	by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 579D15FCC7
	for <reviews@impala.incubator.apache.org>; Fri, 11 Aug 2017 18:13:53 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
	by ip-10-146-233-104.ec2.internal (8.14.4/8.14.4) with ESMTP id v7BIDp7f010615;
	Fri, 11 Aug 2017 18:13:51 GMT
Message-Id: <201708111813.v7BIDp7f010615@ip-10-146-233-104.ec2.internal>
Date: Fri, 11 Aug 2017 18:13:51 +0000
From: "Matthew Jacobs (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>
Reply-To: mj@cloudera.com
X-Gerrit-MessageType: comment
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4833=3A_Compute_precise_per-host_reservation_size=0A?=
X-Gerrit-Change-Id: Idbcd1e9b1be14edc4017b4907e83f9d56059fbac
X-Gerrit-ChangeURL: <http://gerrit.cloudera.org:8080/7630>
X-Gerrit-Commit: af566bb8ccc710069e9981f1a0e60cf8416cfa38
In-Reply-To: <gerrit.1502319910000.Idbcd1e9b1be14edc4017b4907e83f9d56059fbac@gerrit.cloudera.org>
References: <gerrit.1502319910000.Idbcd1e9b1be14edc4017b4907e83f9d56059fbac@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: Fri, 11 Aug 2017 18:13:58 -0000

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4833: Compute precise per-host reservation size
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

Line 63:     //const vector<const FInstanceExecParams*>& instance_params_list,
> Remove?
Done


http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:

PS2, Line 55: FInstanceExecParams*
> Maybe make it const& since it can't be null?
Can't do it without some other trickery: https://stackoverflow.com/questions/10531010/const-references-in-stdvector-elements


Line 251:   /// TODO: Replace uses of unique_hosts_ with per_backend_exec_params.
> It looks like there are a limited number of callsites, can we just do this 
Done


Line 251:   /// TODO: Replace uses of unique_hosts_ with per_backend_exec_params.
> It looks like there are a limited number of callsites, can we just do this 
Done


http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

Line 688:   // TODO: Move to admission control, it doesn't need to be in the Scheduler.
> What if admission control is disabled?
We don't really need the request pool for anything else anymore. It used to live here because the Llama path used it as well, and the Llama/YARN reservation happened outside of AC. That said, if I move this I'd try not to change the behavior, i.e. even if admission control is disabled I'll still resolve the request pool since we do that today.


http://gerrit.cloudera.org:8080/#/c/7630/2/tests/custom_cluster/test_mem_reservations.py
File tests/custom_cluster/test_mem_reservations.py:

Line 38:     config_map = {'DEFAULT_SPILLABLE_BUFFER_SIZE': '33554432',
> The choice of queries, params, etc needs some explanation.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idbcd1e9b1be14edc4017b4907e83f9d56059fbac
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes