dashmap icon indicating copy to clipboard operation
dashmap copied to clipboard

feat req: `try_insert`

Open gitmalong opened this issue 2 years ago • 2 comments

When debugging deadlocks one may want to get more insights and check if a key is locked before doing an insert. Currently this can be done with

match map.try_get_mut(&key) {
    dashmap::try_result::TryResult::Present(mut v) => v.0 = value.0,
    dashmap::try_result::TryResult::Absent => {
        liq_pool_store.insert(key, value); // Deadlock can still occur as another thread could have acquired a ref
    }
    dashmap::try_result::TryResult::Locked => error!("Skipped insert due to lock, key {}", &key),
};

A nicer approach with less boilerplate and no-deadlock guarantee could be:

match map.try_insert(key.clone(), value) {
    dashmap::try_result::TryInsertResult::Inserted => info!("Inserted value for key {}", key),
    dashmap::try_result::TryInsertResult::Locked => error!("Skipped insert due to lock, key {}", &key),
}

Example use case: When facing a deadlock and switching from insert(k,v) to try_insert(k,v) as shown above and no key is repeatedly printed as locked it becomes unreasonable why the dead lock occurs at all (if the remaining code in the block does not interact with the map). Is that assumption right?

gitmalong avatar Jan 01 '23 12:01 gitmalong

How would you consolidate this with the try_insert that already exists on HashMap that does:

Tries to insert a key-value pair into the map, and returns a mutable reference to the value in the entry. If the map already had this key present, nothing is updated, and an error containing the occupied entry and the value is returned.

Alternatively, would using try_entry(&'a self, key: K) -> Option<Entry<'a, K, V, S>> work? Since Entry itself contains a RWLockWriteGuard over the shard that the entry should be in, I think that should prevent deadlock right? Since there isn't a moment like in the first example where you don't have a lock over the map?

I'm thinking of something like

match map.try_entry(key) {
  None => error!("Skipped insert due to lock, key: {}", key),
  Some(entry) => {
    entry.and_modify(|e| e.0 = value.0).or_insert(value);
  }
}

imagine-hussain avatar Mar 13 '23 08:03 imagine-hussain

Please implement this

mscofield0 avatar May 13 '23 14:05 mscofield0