impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4494: Fix crash in SimpleScheduler
Date Fri, 18 Nov 2016 18:53:55 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4494: Fix crash in SimpleScheduler
......................................................................


Patch Set 4:

(2 comments)

Just saw PS4 (which invalidates some of my PS3 comments).

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

Line 261:     if (new_backend_config->NumBackends() > 0
I'm not sure about the NumBackends() > 0 check.

I think the only case when we have an empty set of backends now is when there are no other
registered backends, which means the cluster includes only the current impalad. In that case
I think we want the backend config to include the current backend, rather than waiting for
the change to propagate.


Line 264:       new_backend_config->AddBackend(local_backend_descriptor_);
Do we also need to add this to current_membership_? I think  we want current_membership_ and
the BackendConfig to be consistent.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e1196a2fa47e5954c4a190aa326c135d039a77f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message