-
-
Notifications
You must be signed in to change notification settings - Fork 29
feature: hard delete by tag operation #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
| ]).then(async ([isHardDeleted, isTagInvalidated]) => { | ||
| if (isHardDeleted) { | ||
| // Immediately delete from all layers and return false | ||
| await this.#deleteFromAllLayers(item.entry.getKey()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super happy about performing a side-effect in a method which main purpose is to return a boolean indicating whether an entry is still valid.
If you have a better idea let me know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge problem imo. We are also doing this in isTagInvalidated. But agree with you that its semantically incorrect
|
Thanks a lot for the PR and sorry forlate reply, i have been pretty busy these last weeks... Just a quick ( and maybe naive) question: instead of doing a double round-trip to check hard and soft tag keys separately, what about storing some metadata inside the tag value? Something like: await this.stack.set('___bc:t:users', { timestamp, type: 'hard' | 'soft' }) |
As already outlined in #71, in my opinion the current naming of the
deleteByTagoperation is a bit misleading, since it doesn't hard delete cache entries by tag matching, but rather expires them. That's why I included a commit in this PR renaming the currentdeleteByTagoperation toexpireByTagto be compliant with the naming of the key based operations.I'm aware that this means a breaking change in the API and I'm open to discuss how to resolve this if a major release is to avoid.
The main concern of this PR is the implementation of a new operation
deleteByTag, which performs a "kind of" hard delete via a lazy deletion logic via invalidation timestamps similar to the approach of theexpireByTag(previouslydeleteByTag) operation.Here I'm also open to discuss the best approach for the hard deletion logic.