dragonflydb / dragonfly

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

Handle PreUpdate() and PostUpdate() in PerformDeletion() #2316

Open chakaz opened 6 months ago

chakaz commented 6 months ago
          I think we are missing a preupdate call on the scan items. If we expire the entry while scanning than how do we make sure we call PreUpdate before we delete it?

_Originally posted by @adiholden in https://github.com/dragonflydb/dragonfly/pull/2308#discussion_r1431295495_

adiholden commented 6 months ago

Eviction and Expiring items on background process

chakaz commented 6 months ago

@adiholden and I discussed this a few days ago. There is no correctness issue (we can think of?) when not calling PreUpdate() (and specifically the callbacks) before deleting an item. Usually before updating some value we want replication to send the original bucket, as the update can be incremental. But not sending the bucket before deletion would just mean that the replica will get the bucket without the deleted key. No harm done, unless the bucket is somehow changed by that, which shouldn't(?) be the case. Anyway, it does sound like a good idea to call PreUpdate() for consistency and possibly some edge cases we couldn't think of.