Luo Chen has posted comments on this change.

Change subject: [ASTERIXDB-2186][STO] Cache-friendly Bloom Filter
......................................................................


Patch Set 2:

(3 comments)

Thanks for the comments!

https://asterix-gerrit.ics.uci.edu/#/c/2201/2/hyracks-fullstack/hyracks/hyracks-storage-am-bloomfilter/src/main/java/org/apache/hyracks/storage/am/bloomfilter/impls/BloomFilter.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-bloomfilter/src/main/java/org/apache/hyracks/storage/am/bloomfilter/impls/BloomFilter.java:

PS2, Line 45: 
            :     private static final int DEFAULT_BLOOM_FILTER_VERSION = 0;
            : 
            :     private static final int BLOCKED_BLOOM_FILTER_VERSION = 1;
> Interesting, why's this different than the existing storage version? And ho
The old bloom filter didn't store this version information. And because of the 
initialization of buffer, the version is automatically 0...But right now we 
store it as 1.


PS2, Line 142: 
             :         boolean unpinWhenExit = false;
> Why this and pagesPinned? Why're they different?
I want to expose an API (pinAllPages) to the caller such that a query can pin 
all pages at the beginning, and then call contains, and finally call 
unpinAllPages. Since bloom filter is very small, this avoids the overhead of 
pin/unpin pages every time during a contains call (which leads to another cache 
miss...)

However, if the query didn't call pinAllPages initially, it still works as 
before.


PS2, Line 247: throw 
HyracksDataException.create(ErrorCode.CANNOT_DEACTIVATE_PINNED_BLOOM_FILTER);
> Why is this the case, that it can't be evicted? Won't this possibly interfe
pinAllPages/unpinAllPages should be called by the query (e.g., a lot of pk 
lookups after search secondary index). If a bloom filter is being used by a 
query, it would be a bad idea to evict it's pages...


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2201
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e8e0db9b60d5addfaf61ebb372a1bcb2d2d5957
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-HasComments: Yes

Reply via email to