Skip to content

fix: do not notify watchers when deleting an absent key - #21

Merged
nmbrone merged 1 commit into
mainfrom
fix/delete-absent-key
Sep 2, 2026
Merged

fix: do not notify watchers when deleting an absent key#21
nmbrone merged 1 commit into
mainfrom
fix/delete-absent-key

Conversation

@nmbrone

@nmbrone nmbrone commented Sep 1, 2026

Copy link
Copy Markdown
Owner

delete/2 broadcast {:deleted, bucket, key} unconditionally, so watchers were woken for keys that had never been in the bucket. It was also inconsistent with put_new/3, which stays quiet when it does not write.

:ets.take/2 makes the removal and the presence check one atomic operation, so the broadcast can be conditional at no extra cost over the previous :ets.delete/2.

Behaviour change: a watcher that relied on receiving {:deleted, …} for a key that was not present will no longer get that message.

Covered by a test asserting no notification is sent for an absent key.

Comment thread lib/rkv.ex Outdated
Comment on lines +165 to +166
Watchers are notified only when the key was actually present.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Watchers are notified only when the key was actually present.

@nmbrone
nmbrone force-pushed the fix/delete-absent-key branch from ffe69c9 to 994dee6 Compare September 2, 2026 07:06
delete/2 broadcast {:deleted, bucket, key} unconditionally, so watchers
were woken for keys that had never been in the bucket. This was also
inconsistent with put_new/3, which stays quiet when it does not write.

Use :ets.take/2 so the removal and the presence check are one atomic
operation, and broadcast only when a key was actually removed.
@nmbrone
nmbrone force-pushed the fix/delete-absent-key branch from 994dee6 to 4457e69 Compare September 2, 2026 07:08
@nmbrone
nmbrone merged commit c2302b3 into main Sep 2, 2026
1 check passed
@nmbrone
nmbrone deleted the fix/delete-absent-key branch September 2, 2026 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant