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

EZP-30823: [7.x] Isolated cache during transactions to avoid race condition #2749

Merged
merged 6 commits into from
Sep 30, 2019

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Aug 26, 2019

Question Answer
JIRA issue EZP-30823
Bug/Improvement yes
New feature no
Target version 7.5+, but depends on #2740 being merged first as this is for conflict resolution during merge
BC breaks no
Tests pass yes
Doc needed no

During attempt to backport the transaction isolation fixes done in #2703 a few cases where we should go a bit further appeared:

  • Avoid stale data within transactions:
    • Mark items that has deferred delete/invalidation as a "cache miss" when retrieved from shared pool
    • Merge in in-memory handing in order to make sure in-memory cache is always cleared directly and not deferred to avoid using stal
  • Avoid creating potentially stale data in shared cache pool:
    • Handle save during transaction => map to a deferred delete by key
    • Cache should only be updated when outer transaction finishes, inner transactions don't show up for other processes.
    • Rollbacks should not commit changes. And in SQL they break out of any level transaction by default, align with that
  • BENEFIT: Take advantage of this to get rid of full cache clearing during rollback

REVIEW NOTES:

  • The two Decorators we had has been merged into 1 in order to be able to handle in-memory cache during transaction changes

REVIEW QUESTION:

  • Should we keep ezpublish.cache_pool.inner and instead remove the decoration?
  • Ideally should have something to test this across processes, maybe something like a test with transaction which calls on another process to check it's value while still within transaction. If so, how and where?

@andrerom andrerom requested review from Nattfarinn and webhdx August 26, 2019 09:37
@andrerom andrerom requested a review from alongosz September 1, 2019 12:52
@micszo micszo self-assigned this Sep 9, 2019
@alongosz alongosz changed the title EZP-30823: [7.x] Isolate cache during transactions to avoid race condition EZP-30823: [7.x] Isolated cache during transactions to avoid race condition Sep 10, 2019
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some remarks:

@andrerom andrerom force-pushed the EZP-30823_7.5_transaction_cache_isolation branch 2 times, most recently from fcac772 to 82ceaf6 Compare September 23, 2019 08:39
andrerom added a commit that referenced this pull request Sep 24, 2019
NB! this merge removes all changes done in #2740 as 7.x changes for this is handled seperatly in #2749.
@andrerom andrerom force-pushed the EZP-30823_7.5_transaction_cache_isolation branch from 10fc790 to df6464c Compare September 24, 2019 10:46
@andrerom
Copy link
Contributor Author

andrerom commented Sep 24, 2019

@webhdx What's your POV/Review here?

@andrerom andrerom merged commit 467086a into 7.5 Sep 30, 2019
@andrerom andrerom deleted the EZP-30823_7.5_transaction_cache_isolation branch September 30, 2019 14:54
@micszo micszo removed their assignment Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants