impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3502: Fix race in the coordinator while updating filter routing table
Date Wed, 11 May 2016 18:25:42 GMT
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 <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message