feat: impl singleflight #105
Conversation
Signed-off-by: tison <wander4096@gmail.com>
| /// units of work can be executed with duplicate suppression. | ||
| #[derive(Debug)] | ||
| pub struct Group<K, V> { | ||
| map: Mutex<HashMap<K, Arc<OnceCell<V>>>>, |
There was a problem hiding this comment.
This can be improved with more efficient concurrent hashmap. But Go's impl use simple Mutext as well so I think it's generally OK.
We can leave using other hashmap impls (and thus extra deps) a follow-up.
| let res = cell | ||
| .get_or_init(async || { | ||
| // I am the leader. | ||
| let result = func().await; | ||
|
|
||
| // Cleanup: remove the key from the map. | ||
| // We must ensure we remove the entry corresponding to *this* cell. | ||
| let mut map = self.map.lock(); | ||
| if let Some(existing) = map.get(&key) { | ||
| // Check if the map still points to our cell. | ||
| if Arc::ptr_eq(&cell, existing) { | ||
| map.remove(&key); | ||
| } | ||
| } | ||
|
|
||
| result | ||
| }) | ||
| .await; | ||
|
|
||
| res.clone() |
There was a problem hiding this comment.
An alternative impl method is to build the sync block manually, with potentially registered oneshot channels.
In that case, we can reduce one .clone() if there is no other waiter, and the wakeup logic can be done in batch.
There was a problem hiding this comment.
I investigate the OnceCell impl in mea, find some potential problems:
1. leader computed result
2. leader remove key from `Group.map`
3. but OnceCell not yet soon set_value, value_set still false
4. new coming calls see key not in map -> create a new OnceCell -> become new leader
There was a problem hiding this comment.
BTW: This is not a concurrency safety issue, but a semantic safety issue.
There was a problem hiding this comment.
Let me see ...
I've tried the onshot channels solution, but that can hardly handle if the first leader panics. So I tend to keep the OnceCell solution now at the cost of one extra clone of the result.
There was a problem hiding this comment.
I investigate the
OnceCellimpl in mea, find some potential problems:1. leader computed result 2. leader remove key from `Group.map` 3. but OnceCell not yet soon set_value, value_set still false 4. new coming calls see key not in map -> create a new OnceCell -> become new leader
This should not be an issue because you can consider the call comes at step 4 "happen after" the result was set and delivered.
Singleflight is not a cache, for sure.
|
I'm going to merge this patch this week and release 0.6.1. Even if we do some internal refactor, the public API should remain the same. |
|
Merged. I'd stay some time for thinking about it once more. @ariesdevil feels free to ping me for a release when you need it. |
This closes #104.
cc @ariesdevil