Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/15308 )
Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled ...................................................................... Patch Set 2: (1 comment) Hi Fang-Yu, thanks for your feedback. I replied to your comment below. http://gerrit.cloudera.org:8080/#/c/15308/2/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/15308/2/be/src/runtime/coordinator.cc@1268 PS2, Line 1268: state->set_all_update_received(); > I think by calling "state->set_all_update_received();" here (after the call I think I agree with your assessments for point 1 to 5. Regarding your last concern, I now realize that if ApplyUpdate() caught in RPC failure, the filter still get published as always_true filter. So it is probably better to move this line before Disable() but with precondition that the filter is not yet disabled, like this: if (!state->disabled()) state->set_all_update_received(); state->Disable(); Thus, case number 2 and 3, where there is an RPC error in ApplyUpdate(), will not be marked as complete runtime filter. -- To view, visit http://gerrit.cloudera.org:8080/15308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef Gerrit-Change-Number: 15308 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 04 Mar 2020 21:18:51 +0000 Gerrit-HasComments: Yes
