al8n / stretto

Stretto is a Rust implementation for Dgraph's ristretto (https://github.com/dgraph-io/ristretto). A high performance memory-bound Rust cache.
Apache License 2.0
412 stars 28 forks source link

Can you use a cache after clear()? #18

Closed dmrolfs closed 2 years ago

dmrolfs commented 2 years ago

Should you be able to insert into the cache after you clear() it? The test case below fails on the final line since the get() returns None (after insert and wait). Looking at the code, I believe this is a bug and not a design decision; however, I don't see an attempt to use a cache after clear in the examples or tests.

    use claim::*;
    use pretty_assertions::assert_eq;

    #[test]
    fn test_basic_cache_add_after_clear() {
        let ttl = Duration::from_secs(60);
        block_on(async {
            let cache = AsyncCache::builder(1000, 100)
                .set_metrics(true)
                .set_ignore_internal_cost(true)
                .finalize()
                .expect("failed creating cache");

            assert!(cache.insert_with_ttl("foo".to_string(), 17.to_string(), 1, ttl).await);
            assert!(cache.insert_with_ttl("bar".to_string(), "otis".to_string(), 1, ttl).await);
            assert_ok!(cache.wait().await);

            assert_eq!(assert_some!(cache.get(&"foo".to_string())).value(), &17.to_string());
            assert_eq!(assert_some!(cache.get(&"bar".to_string())).value(), &"otis".to_string());

            assert_ok!(cache.clear());
            assert_ok!(cache.wait().await);

            assert_none!(cache.get(&"foo".to_string()));

            assert!(cache.insert_with_ttl("zed".to_string(), 33.to_string(), 1, ttl).await);
            assert_ok!(cache.wait().await);

            assert_none!(cache.get(&"bar".to_string()));
            assert_eq!(assert_some!(cache.get(&"zed".to_string())).value(), &33.to_string());
        });
    }

error message in my project:

---- phases::sense::clearinghouse::cache::tests::test_basic_cache_add_after_clear stdout ----
thread 'phases::sense::clearinghouse::cache::tests::test_basic_cache_add_after_clear' panicked at 
'assertion failed, expected Some(..), got None', src/phases/sense/clearinghouse/cache.rs:272:24
al8n commented 2 years ago

Thanks, @dmrolfs. This is a bug and only happens in AsyncCache. I have tested your code and found that sometimes it gets the correct result but sometimes it fails.

goldwind-ting commented 2 years ago

Should you be able to insert into the cache after you clear() it? The test case below fails on the final line since the get() returns None (after insert and wait). Looking at the code, I believe this is a bug and not a design decision; however, I don't see an attempt to use a cache after clear in the examples or tests.

    use claim::*;
    use pretty_assertions::assert_eq;

    #[test]
    fn test_basic_cache_add_after_clear() {
        let ttl = Duration::from_secs(60);
        block_on(async {
            let cache = AsyncCache::builder(1000, 100)
                .set_metrics(true)
                .set_ignore_internal_cost(true)
                .finalize()
                .expect("failed creating cache");

            assert!(cache.insert_with_ttl("foo".to_string(), 17.to_string(), 1, ttl).await);
            assert!(cache.insert_with_ttl("bar".to_string(), "otis".to_string(), 1, ttl).await);
            assert_ok!(cache.wait().await);

            assert_eq!(assert_some!(cache.get(&"foo".to_string())).value(), &17.to_string());
            assert_eq!(assert_some!(cache.get(&"bar".to_string())).value(), &"otis".to_string());

            assert_ok!(cache.clear());
            assert_ok!(cache.wait().await);

            assert_none!(cache.get(&"foo".to_string()));

            assert!(cache.insert_with_ttl("zed".to_string(), 33.to_string(), 1, ttl).await);
            assert_ok!(cache.wait().await);

            assert_none!(cache.get(&"bar".to_string()));
            assert_eq!(assert_some!(cache.get(&"zed".to_string())).value(), &33.to_string());
        });
    }

error message in my project:

---- phases::sense::clearinghouse::cache::tests::test_basic_cache_add_after_clear stdout ----
thread 'phases::sense::clearinghouse::cache::tests::test_basic_cache_add_after_clear' panicked at 
'assertion failed, expected Some(..), got None', src/phases/sense/clearinghouse/cache.rs:272:24

This is because the clear is still running when you insert zed.