Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201703231828.v2NISR5F027960@ip-10-146-233-104.ec2.internal>
Date: Thu, 23 Mar 2017 18:28:27 +0000
From: "Henry Robinson (Code Review)" <gerrit@cloudera.org>
To: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Matthew Jacobs <mj@cloudera.com>, Marcel Kornacker <marcel@cloudera.com>
Reply-To: henry@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4041=3A_Limit_catalog_and_admission_control_updates_to_coordinators=0A?=
In-Reply-To: <gerrit.1489173273000.I5f2c74abdbcd60ac050efa323616bd41182ceff3@gerrit.cloudera.org>
References: <gerrit.1489173273000.I5f2c74abdbcd60ac050efa323616bd41182ceff3@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: Thu, 23 Mar 2017 18:28:34 -0000

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4041: Limit catalog and admission control updates to coordinators
......................................................................


Patch Set 4:

(9 comments)

Please consider adding something to the root web page (and to the log) saying if an Impala is running in coordinator mode.

http://gerrit.cloudera.org:8080/#/c/6344/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 1588:     LOG(WARNING) << "Failed to convert hostname " << hostname << " to IP address";
log status?


Line 1589:     return;
> do you know what the effect of this will be? it's a bit weird not to be abl
It means that the backend won't be available for execution. It's possible that FLAGS_hostname is not set correctly, but also that there's a DNS outage or some other problem.


PS4, Line 1601: address
nit: descriptor


PS4, Line 1604: }
              :   if (status.ok()) {
else { 

}


PS4, Line 1909: NULL
nullptr


PS4, Line 1931: NULL
nullptr


http://gerrit.cloudera.org:8080/#/c/6344/4/be/src/service/impala-server.h
File be/src/service/impala-server.h:

PS4, Line 438: In
nit: should be To, not In


http://gerrit.cloudera.org:8080/#/c/6344/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

PS4, Line 40: 3
is there any way to make this default == the value of '-s'? If I run -s1 right now, won't this fail the check on line 340? If not, I think removing that check is fine - doesn't matter if we ask for more coordinators than we get.


http://gerrit.cloudera.org:8080/#/c/6344/4/tests/custom_cluster/test_coordinators.py
File tests/custom_cluster/test_coordinators.py:

Line 83:       client2.close()
can you add a check that the worker has not received the catalog metadata? GET /catalog or something.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f2c74abdbcd60ac050efa323616bd41182ceff3
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes