impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4041: Limit catalog and admission control updates to coordinators
Date Fri, 24 Mar 2017 21:27:19 GMT
Dimitris Tsirogiannis has posted comments on this change.

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


Patch Set 4:

(10 comments)

I am not so sure about removing the catalog and in-flight queries handles. They provide a
way of visually validating that worker nodes do not receive catalog updates and that they
don't schedule queries.

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

Line 259
> This line is effectively removed, right? It was added in: https://gerrit.cl
After inspecting the code and also talking to Lars, I believe this removal is safe. This change
has the following effect. The scheduler has to wait for the statestore update that includes
the local backend in order to add it to the backend config. Before that, the backend_config
could be empty and scheduling should fail unless exec_at_coord is true. We have tests for
these cases.


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


PS4, Line 1601: address
> nit: descriptor
Done


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


PS4, Line 1909: NULL
> nullptr
Done


PS4, Line 1931: NULL
> nullptr
Done


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

Line 436:   // it to the list of known backends and tell the statestore by adding it to
> this function isn't telling the statestore anything, it's only adding to to
Done


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


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 ri
Hm, not sure how to do that. I removed the check.


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


-- 
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: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@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

Mime
View raw message