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 <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@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