search

moka-rs / mini-moka

A simple concurrent caching library that might fit to many use cases
Apache License 2.0
105 stars 6 forks source link

Brush up the eviction handler implementation #14

Open tatsuya6502 opened 1 year ago

tatsuya6502 commented 1 year ago

As of now (commit #f7a15e24), the following issues might be present with the eviction handler:

  1. invalidate an existing entry will always trigger the eviction handler with RemovalCause::Explicit.
    • a. But there are chances that entry has been already expired. In such case, the eviction handler should be triggered with RemovalCause::Expired.
    • b. The same thing applies to invalidate_all.
  2. There is no code path for RemovalCause::Replaced.
    • a. We should also select RemovalCause::Replaced or RemovalCause::Expired based on the expiration status of the entry.

For 1-a and 2-a, see moka's this unit test: moka sync/cache.rs#L4270-L4280

tatsuya6502 commented 1 year ago

Also an internal do_upsert_with_hash method should check whether the existing entry is expired or not. If expired, it should skip to call the update closure.

https://github.com/moka-rs/mini-moka/blob/f7a15e24211c3d7182d6f9b83ee4f601bde8c567/src/sync/base_cache.rs#L295-L296

tatsuya6502 commented 1 year ago

Since this feature is not released yet, I am setting enhancement label rather than bug.