-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
[RFC]: Removal of increment/decrement functionality #117
Comments
I support the proposal, adding some important thought: What is a cache increment/decrement supposed to do? Add or subtract 1 from a value. Having 1000 concurrent requests incrementing a value better ends up having the value incremented by 1000, but concurrency and some backend's inability to provide this operation atomically will prevent it from happening. Several backends like APC(u), Memcache(d), Redis do provide an atomic increment operation on the server, which can be triggered, and I am confident the result with concurrent requests will be correct. Using the filesystem, MongoDB or DBA backend however, this will definitely fail. Or would require some locking mechanism to be available to prevent other processes to access the value being worked on, but not all backends even support them natively. Locking also is alien to a caching layer, it is supposed to provide write-once-read-many operations to speed up things, not make requests wait for each other. If software requires distributed access to counters, it should use dedicated access layers for this task - not using a generic caching layer with interesting additional features. Requiring reliable counters probably means you have to familiarize yourself with the counter providing software anyways. |
Moving this to a dedicated interface might be the way to go. As of now it is implicitly provided by the |
Another little "fun-fact" is that there are adapters where I really tend to remove this feature. |
As per laminas#117, the increment and decrement functionality (at least decrement) leads to issues with several backends. This commit removes that functionality at all as it should be either part of specific adapters instead (via dedicated interface) or as part of project specific code. This cache component can not guarantee increment/decrement for all adapters out there (especially for adapters which do not support this feature natively to prevent race conditions). Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
As per laminas#117, the increment and decrement functionality (at least decrement) leads to issues with several backends. This commit removes that functionality at all as it should be either part of specific adapters instead (via dedicated interface) or as part of project specific code. This cache component can not guarantee increment/decrement for all adapters out there (especially for adapters which do not support this feature natively to prevent race conditions). Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Removed with #294 |
RFC
Goal
It seems that some storage backends (
redis
for example) does not preserve type-safety when storing values.Incrementing and decrementing of cache items is therefore mostly done by this library by reading the value, Incrementing/decrementing the value and then it is written back to the storage.
This could also be done in userland code and/or by a decorator and thus does not need a requirement for a storage anymore.
Background
Recently, there were 2 bugs discovered when using the serializer plugin in combination with decrement/increment which were undetected for around 9 years.
Considerations
Providing a decorator which consumes the storage along with the key & amount could provide the same functionality without having every storage do handle it by itself.
Proposal(s)
Thus said, I would mark that whole functionality deprecated in 3.0 and remove it in 4.0
Decorator to increment/decrement could be created in 3.0 if needed. The whole functionality could be done in userland without having to use a decorator at all and thus the decorator could also be just an example in the docs.
Appendix
Bugs mentioned in this RFC which are related to this functionality:
#116
#115
The text was updated successfully, but these errors were encountered: