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 <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to