feat: support cuckoo filter and most commands #2545
Conversation
…into cuckoo-filter
|
Running |
|
@jonathanc-n Thank you for this contribution. Great job! |
|
Thanks for your contribution! However, I cannot find a proposal to state the design details and storage format of the new data structure. May I ask you provide such a proposal before we continue to review this patch? |
|
@PragmaTwice it is mentioned in #2539, I added some extra information to clarify some things. |
|
|
||
| /* | ||
| This project uses Redis, which is licensed under your choice of the | ||
| Redis Source Available License 2.0 (RSALv2) or the Server Side Public License v1 (SSPLv1). |
There was a problem hiding this comment.
As an ASF project, we CANNOT use any code under these non-open-source licenses, which includes RSAL and SSPL.
| for (const auto &filter : filters) { | ||
| PutFixed16(dst, filter.bucket_size); | ||
| PutFixed64(dst, filter.num_buckets); | ||
| dst->append(filter.data.begin(), filter.data.end()); |
There was a problem hiding this comment.
I doubt the efficiency of this approach: everytime we read/update the filters, we need to retrieve/put all of these filters.
Please refer to the design of bloom filters to see if you can put filters to multiple key values.
|
@PragmaTwice Sorry, about this implementation, are you able to close this pr. I might come back to it with a completely different code implementation. |
|
Sure. |
references issue: #2534
Added support for all commands except for SCANDUMP and LOADCHUNK.
The go cases are currently being worked on.