search

dragonflydb / dragonfly

A modern replacement for Redis and Memcached
https://www.dragonflydb.io/
Other
25.44k stars 919 forks source link

Enable tx inline execution when snapshoting/journaling step 1 - transaction acquire key lock on OPTIMISTIC_EXECUTION #3841

Open adiholden opened 2 hours ago

adiholden commented 2 hours ago

Today we do not acquire key lock when transaction runs inline (OPTIMISTIC_EXECUTION) If we preemp when running the command callback execution this can lead to a bug We need this change to enable inline schedualing while replicating/snapshoting/using tiering which can cause preemption in callback execution

kostasrim commented 2 hours ago

uh I remember I had this discussion with @romange while I was working on big value serialization

IMO I always thought that inline transactions do not require to acquire a lock for the keys since they are "fast to execute" but preemptions do cause issues.

Also in the past, we did require locks even for inline transactions so this PR is as simple as reverting those changes.

I do wonder what's the overhead of acquiring those locks. I guess there should be none, since they are "logical locks". Maybe worth benchmarking. (again I don't remember the code there so I will take a look and maybe add one if it's worth it -- whcich IMO I don't think that's the case)

adiholden commented 1 hour ago

The code evolved alot I dont think you can just revert the changes, and I dont think benchmarking for this change this should not have impact on performance

romange commented 57 minutes ago

Yeah, I recently optimized the data structures there (we use fingerprints now), so I guess it should have very negligible impact.