Tim Armstrong has posted comments on this change. Change subject: IMPALA-4494: Fix crash in SimpleScheduler ......................................................................
Patch Set 3: (4 comments) It would still be nice to remove the situation between the first topic update and the second where we have all of the backends except the local one. I feel like that scenario just makes things hard to reason about. http://gerrit.cloudera.org:8080/#/c/5127/3/be/src/scheduling/simple-scheduler.cc File be/src/scheduling/simple-scheduler.cc: Line 204: if (delta.is_delta && delta.topic_entries.empty() && delta.topic_deletions.empty()) { Can you comment on what this achieves (it's an optimisation to avoid a copy rather than a correctness thing, right?) Line 251: new_backend_config->AddBackend(be_desc); Maybe we should just ignore updates for the local backend here. Line 255: for (const string& backend_id: delta.topic_deletions) { And here Line 265: if (current_membership_.find(local_backend_id_) == current_membership_.end()) { And then add the local backend directly instead of waiting for it to propagate back from the statestore. -- 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: 3 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
