Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
......................................................................


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@139
PS10, Line 139: esc.filter_id);
> If the reservation is claimed, then it is considered a fatal error if alloc
I think what you mean is, it is ok to allocate later as long as the whole 
total_bloom_filter_mem_required_ is already claimed. Is that correct?

ps14 move the initialization to UpdateFilterFromRemote().


http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@722
PS10, Line 722: HECK_EQ(0, produced_filter
> Few things about this:
If this can be a reassurrance, note that SendIncompleteFilters is only called 
when RuntimeFilterBank is closing.
RuntimeFilterBank lifetime is equal to query lifetime in that executor node. It 
is closing only if query is completed, or canceled. On both case, plan root 
sink is basically done, and runtime filter value does not matter anymore. 
Coordinator can just drop runtime filter update by then.

CombinePeerAndLocalUpdates() is done here for correctness. It cleanup 
'pending_merge_filter' and 'pending_remote_filter' of 'produced_filter'.

This feature should be exercised in TestRuntimeFilters, TestBloomFilters, 
TestBloomFiltersOnParquet, and TestRuntimeRowFilters. And 
test_wait_time_cancellation is within TestRuntimeFilters.



--
To view, visit http://gerrit.cloudera.org:8080/20612
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Wed, 13 Dec 2023 20:31:34 +0000
Gerrit-HasComments: Yes

Reply via email to