Skip to content

The cache should validate that found items were actually created by it / meant for it #22

Open

Description

@naderman
Q A
Bug? yes
New Feature? no
Version

Actual Behavior

The cache uses any item matching the key and may error if the content does not match what it expects.

Expected Behavior / Possible Solution

The cache should treat an item that was potentially created by another service with a colliding key as a reason to skip caching the resource with this key.

Activity

  1. Nyholm commented on Oct 14, 2016

    @Nyholm
    Member

    Another possible solution would be to treat that cache data as invalid/empty and overwrite it if needed.

  2. naderman commented on Oct 14, 2016

    @naderman
    Author

    That'd be fine with me too, but may lead to two separate systems continuously overwriting the value

  3. Nyholm commented on Oct 14, 2016

    @Nyholm
    Member

    Yeah. That is why one would need the PrefixedCache php-cache/cache#96 =)

    If we do not overwrite we risk that random garbage data from an other service blocks this cache key forever. That would mean that we never can cache.

  4. dbu commented on Oct 15, 2016

    @dbu
    Contributor

    i would vote that we implement overwriting then. if you use a different cache instance per service you can already use different paths, and ifnot the PR @Nyholm did will further improve the situation. still, data on a disk could be corrupted for whatever reason (incomplete write, actual disk failure, other types of "this should never happens" errors) so stopping to cache sounds wrong.

  5. sagikazarmark commented on Oct 15, 2016

    @sagikazarmark
    Member

    I think overwriting should be the way to go.

    @naderman cache libraries and cache systems usually support namespacing, so avoiding collision is the responsibility of the user to configure it IMO.

  6. kelunik commented on Nov 13, 2017

    @kelunik

    IMO it should just fail hard and throw an exception. It's clearly a configuration mistake that needs to be fixed.

  7. dbu commented on Nov 13, 2017

    @dbu
    Contributor

    its an error, yes, but in production mode you rather want things to survive than a hard exception. it can log an error, or if it can know we are in some sort of dev mode, it can throw an exception.

  8. kelunik commented on Nov 13, 2017

    @kelunik

    In that case throw a warning each time and ignore the cache. It ought to be fixed.

  9. dbu commented on Nov 13, 2017

    @dbu
    Contributor

    yeah agree, trigger a warning level error sounds like the right thing to do.

    ignoring the cache in that case could lead to cache stop working at some point down the road if it either got corrupted by a random filesystem accident, or a hard crash mid-writing to the cache, or because we changed the format (maybe by accident, or in a new major version). thus i think overwriting should be ok in that case.

  10. kelunik commented on Nov 13, 2017

    @kelunik

    That's a good point, indeed. 👍

  11. added this to the 3.0 milestone on Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions