Re: [pve-devel] [RFC proxmox 4/7] cache: add new crate 'proxmox-cache'

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Aug 22, 2023 at 01:33:44PM +0200, Lukas Wagner wrote:
(...)
> > ... can be replaced as follows, in order to make it similar to
> > std::collections::{HashMap, BTreeMap}:
> > 
> > impl<K: AsRef<str>> for SharedCache<K, Value> {
> >      // Returns old value on successful insert, if given
> >      fn insert(&self, k: K, v: Value) -> Result<Option<Value>, Error> {
> >          // ...
> >      }
> > 
> >      fn get(&self, k: K) -> Result<Option<Value>, Error> {
> >          // ...
> >      }
> > 
> >      fn remove(&self, k: K) -> Result<Option<Value>, Error> {
> >          // ...
> >      }
> > }
> > 
> > If necessary / sensible, other methods (inspired by {HashMap, BTreeMap} can
> > be added as well, such as remove_entry, retain, clear, etc.
> > 
> 
> I don't have any hard feelings regarding the naming, but not returning a
> Value from `delete` was a conscious decision - we simply don't need it right
> now. I don't want to deserialize to just throw away the value.
> Also, reading *and* deleting at the same time *might* introduce the need for
> file locking - although I'm not completely sure about that yet.
> 
> If we ever need a `remove` that also returns the value, we could just
> introduce a second method, e.g. `take`.

Locking might make sense though.

If we want to refresh data, it may make sense to first lock the cache
entry, then re-check the expiration time, and only then do the actual
refresh.
That way, 5 concurrent accesses will cause only a single refresh, since
afterwards they all see the new value.

As for `take` vs locking: You *could* have the first operation be a
`rename` to a random string, then open,delete,read (but then you need
something that cleans up after left-overs when a process gets killed
between the rename & delete.
But I don't quite see the value in this functionality here for now :-)