Bug in moka::future::Cache::invalidate_all? Elements not being invalidated immediatelly.

[bsdinis]

Hi.

According to the documentation of invalidate_all:

pub fn invalidate_all(&self)
Discards all cached values.

This method returns immediately and a background thread will evict all the cached values inserted before the time when this method was called. It is guaranteed that the get method must not return these invalidated values even if they have not been evicted.

Like the invalidate method, this method does not clear the historic popularity estimator of keys so that it retains the client activities of trying to retrieve an item.

From this I surmised that, even though the actual removal of elements occurs in the background, they would be somehow marked as invalid, and as such a subsequent get would not see them.

However, this does not seem to be happening. Here's a small minimal example that exemplefies this

#[tokio::test]
async fn test_cache_invalidate() {
    let cache = Cache::new(1024 as u64);

    assert_eq!(cache.get(&0), None);
    cache.insert(0, 1).await;
    assert_eq!(cache.get(&0), Some(1));
    cache.invalidate_all();
    assert_eq!(cache.get(&0), None);
}

This fails in the last line (the get returns Some(1))

Is this a bug or am I misreading the documentation?

[tatsuya6502]

Thank you for reporting the bug. I was aware of it when I implemented invalidate_all function, but I have never had a chance to fix it and then forgot.

I fixed the bug with #156, and will release Moka v0.8.6 after I finish running a pre-release testing.

The root cause of the bug was that a cache entry's last_modified timestamp was not set to the current time when it was inserted to the cache. Instead, it was set to the time when the cache entry was properly admitted to the cache by a background thread called "housekeeper".

Because of this, invalidate_all function was unable to invalidate cache entries inserted just before calling invalidate_all.

You can confirm this behavior by modifying your reproducer code as the followings:

#[tokio::test]
async fn invalidate_all_without_sync() {
    let cache = Cache::new(1024);

    assert_eq!(cache.get(&0), None);
    cache.insert(0, 1).await;
    assert_eq!(cache.get(&0), Some(1));

    // In Moka v0.8.5 or earlier, you can workaround the issue
    // by doing one of the followings:
    // 
    // - Call `cache.sync()` before invalidating.
    //     - `cache.sync()` will run pending housekeeping task,
    //        so the entry's `last_modified` will be set to the
    //        current time.
    // - Or, wait some time for the housekeeping task to run.

    // Note that `cache.sync()` can be a heavy operation so it is
    // not recommended to call it very often.
    use moka::sync::ConcurrentCacheExt;
    cache.sync();

    // The housekeeping task will run ~0.5 seconds after the cache
    // creation. Then it will run with ~0.3 seconds interval if the
    // cache is not receiving heavy writes.
    // 
    // tokio::time::sleep(Duration::from_millis(520)).await;

    cache.invalidate_all();
    assert_eq!(cache.get(&0), None);
}

With the next release Moka v0.8.6 (that has the fix), you do not have to do the above workaround. I will let you know when I publish v0.8.6 to crates.io.

[bsdinis]

Thanks so much!

[tatsuya6502]

Hi @bsdinis. I have published Moka v0.8.6 to crates.io with the fix. Thanks!