intel / CacheLib

Pluggable in-process caching engine to build and scale high performance services
https://www.cachelib.org
Apache License 2.0
17 stars 4 forks source link

[draft] initial sync for chained items based on head handle #75

Closed byrnedj closed 1 year ago

byrnedj commented 1 year ago

This change is Reviewable

igchor commented 1 year ago

@byrnedj Can you also write down why we want to synchronize on either parent or head item (I forgot)? Why not just synchronize on the child? I know that this changes the semantics a bit, but I'm just wondering if there are any fundamental reasons we can't do that.

As I'm thinking about this right now, it seems to me that we should be able to make it work. One problem with such approach would be evicting the chain if moving fails (because we cannot use markForEviction when moving, we need to try to mark the parent for eviction). But we can just unmark the child, try to mark the parent and loop again if that fails.

This way, the only thing we would need to do on read path is to modify each loop that iterates over the chain to obtain handle for each child / manually check isMoving(). We could even probably just reuse moveChainedItem if we'd generalize the code for replacingInMMContainer. Something like this: https://github.com/igchor/CacheLib-1/commit/f62b7afb08e2d4a7ef5d4bcba9299388775dc485 (does not compile yet)

We would also need to skip creating the handles for child items in the thread that actually moves the item between chains (otherwise we would have a deadlock - that thread would wait on item that it itself marked as moving) but it also seems doable.

byrnedj commented 1 year ago

This is implemented in https://github.com/intel/CacheLib/pull/84