-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Handle serialization failures for Memcached #23615
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
Conversation
12bd3c2
to
df3cc55
Compare
So, what would be your stand on #19895? |
Accidentally hit delete when editing my original message; here it is again:
I think my initial reaction was an uneducated gut-reaction without fully understanding the intentions of this PR; if your goal is #19895 (comment), then I'm onboard, and may even have some time to contribute towards those goals. |
My goal with this PR is to have a predictable, thus safe, default behavior. :) |
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.
I would merge this as a bug fix in 3.3 instead. Wiping the cache should never be an issue (should probably be done anyway on a new deploy).
df3cc55
to
ddab824
Compare
Rebased to target 3.3 |
ddab824
to
b14b0e1
Compare
b14b0e1
to
cccc88f
Compare
…as-grekas) This PR was merged into the 3.3 branch. Discussion ---------- [Cache] Handle serialization failures for Memcached | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Fixes two issues with serialization + memcached: with the memcached extension, the default serializer is automatically selected as igbinary when possible, native php otherwise. That creates obvious migration/portability issues (ie just installing igbinary wipes out the value of your cache.) Then, handling unserializing failures (esp. "php_incomplete_class") is a paramount feature of the component. You must be able to deal with migrating you code base without being blocked by some legacy serialized data. Commits ------- cccc88f [Cache] Handle unserialization failures for Memcached
Fixes two issues with serialization + memcached: with the memcached extension, the default serializer is automatically selected as igbinary when possible, native php otherwise. That creates obvious migration/portability issues (ie just installing igbinary wipes out the value of your cache.)
Then, handling unserializing failures (esp. "php_incomplete_class") is a paramount feature of the component. You must be able to deal with migrating you code base without being blocked by some legacy serialized data.