Dan Hecht has posted comments on this change. Change subject: IMPALA-3502: Fix race in the coordinator while updating filter routing table ......................................................................
Patch Set 1: (3 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 there a reason for that? Line 424: if (schedule.num_fragment_instances() == 0) MarkFilterRoutingTableComplete(); also here. could we make them all consistent? (the guard either inside the routing table routines or in the caller and then make the the routing table routines DCHECK that filters aren't off). Line 668: if (filter_mode_ != TRuntimeFilterMode::OFF) { how about making this a DCHECK now that the caller does it? see other comments though. -- 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: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
