[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

["Tim Armstrong (Code Review)" <ge...@cloudera.org>]

Tim Armstrong 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 1:

(1 comment)

This seems to solve the problem but I think the implementation could be either simpler or better explained. I left comments inline.

http://gerrit.cloudera.org:8080/#/c/15308/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/15308/1/be/src/runtime/coordinator-backend-state.cc@671
PS1, Line 671:       state->set_all_publish_complete();
On first look, it seemed weird that the runtime filter flips from not disabled to disabled to disabled again in the filter table.

I guess I also don't exactly understand what the intent is. I had initially thought that it might be to show filters where PublishFilter() failed as disabled, but it looks like set_all_publish_complete() is called even if there's an RPC error.

One idea that might simplify things is if in UpdateFilter() we marked the filter as complete at the same time as calling Disable().



-- 
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: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 01:52:28 +0000
Gerrit-HasComments: Yes