Skip to content
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

Don't delay cache additions until after the transaction ends #3172

Merged
merged 5 commits into from
Feb 16, 2024

Commits on Feb 13, 2024

  1. Data::ArtistCredit: Use Data::Role::GIDRedirect

    The methods in question were copied from the `GIDRedirect` role in 8a841d6,
    but I believe we can just use that role directly.
    mwiencek committed Feb 13, 2024
    Configuration menu
    Copy the full SHA
    550ecba View commit details
    Browse the repository at this point in the history
  2. Invalidate deleted/updated GID redirects from the cache

    This will fix test failures in a subsequent commit.
    
    It appears that we don't currently do this, but issues did not arise in the
    tests due to cache additions being delayed (i.e., there being nothing to
    invalidate).
    mwiencek committed Feb 13, 2024
    Configuration menu
    Copy the full SHA
    789689a View commit details
    Browse the repository at this point in the history
  3. Ensure reorder_for_entities runs after cache invalidation

    `automatically_reorder` (invoked by `reorder_for_entities`) sorts series items
    by "part of" relationship, which are loaded via our Perl data models, which in
    turn will load entities on those relationships. Those entities may have been
    updated in the current transaction and thus should be invalidated from the
    cache before we attempt to sort the series items based on stale entity data.
    (This will fix test failures in a subsequent commit.)
    mwiencek committed Feb 13, 2024
    Configuration menu
    Copy the full SHA
    511d895 View commit details
    Browse the repository at this point in the history
  4. Don't delay cache additions until after the transaction ends

    This partially reverts 82c79f5 and
    42be4bc.
    
    After searching for issues similar to MBS-13481 (where we load additional data
    onto an entity which is then entered into the cache, but where changes to that
    additional data do not invalidate the entity's cache entry), I believe that
    there are simply far too many uncovered cases which we'd have to handle.
    
    While storing more data in the cache and having intelligent invalidation logic
    for all the linked data sounds desirable, it's too many changes to manage for
    now, so we need to restore the previous behavior.
    
    I realized (later than I should have) that we don't need to delay cache
    additions until after the database transaction ends. We may have had to in a
    previous implementation I had, but no longer.
    
    The `post_txn_callbacks` ran regardless of whether or not the commit succeeded,
    so an entity would always be cached in the state it was initially loaded in.
    The only difference now is when that occurs. There's no type of race condition
    that isn't already handled:
    
     * If the entity is deleted from the cache right after it's added, then there
       is no issue, just like before.
    
     * If the entity is added to the cache right after it's deleted, then there is
       still no issue, because our existing code to delete the added entry if there
       exists a `recently_invalidated` key for the same still applies.
    
    The existing `*EntityCache` tests pass, and I've added a couple additional
    tests to ensure that neither release events nor release labels are entered into
    the cache with a release.
    mwiencek committed Feb 13, 2024
    Configuration menu
    Copy the full SHA
    05ae5c1 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    49fa867 View commit details
    Browse the repository at this point in the history