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 <l...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
