Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3502: Fix race in the coordinator while updating filter routing table ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/3018/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 423: UpdateFilterRoutingTable(request.fragments[0].plan.nodes, 1, 0); > seems kind of weird that this doesn't have the OFF guard around it. is ther Done Line 424: if (schedule.num_fragment_instances() == 0) MarkFilterRoutingTableComplete(); > also here. could we make them all consistent? (the guard either inside the Added the OFF guard and replaced it inside the MarkFilter... function with a DHECK. Done Line 547: // Populate the runtime filter routing table > Comment that this needs to be done before starting the remote fragments. Done Line 563: (has_coordinator_fragment ? 1 : 0) > It would be good to factor this out into first_remote_fragment_idx, or some Done Line 668: if (filter_mode_ != TRuntimeFilterMode::OFF) { > how about making this a DCHECK now that the caller does it? see other comme Done -- To view, visit http://gerrit.cloudera.org:8080/3018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecc737106fd38aa4af0c72959a577adfb413728d Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
