L2 cache save operation stop working when the hash key exists on remo… #36418

georgebabarus · 2022-11-03T17:07:35Z

Save operation stop working on L2 cache and remote cache when the hash key exists on remote server, but the data key was evicted meanwhile from remote cache.

Later edit: The original issue has been fixed int magento:2.4-develop branch, but this is a performance improvement implementation that should be beneficial by following argument: The concept of cache involves try loading the data from storage, generate if data doesn't exist in cache, and save it to cache. There is no need to check if data exist in cache in the save method.

Description (*)

When save method is called on L2 cache adapter there are 3 things do be done:

update the data on remote server (this will make sure the data exists on the remote server and the TTL is renewed)
update the version (hash) on the remote server (this will make sure the version id (hash) exists on the remote server and the TTL is renewed)
update the data in the local cache (this will make sure the data exists on the local server)

The original checks of the remote version of data are preventing the save of the cache to remote and local servers in the scenario where the version id exists and is correct but the data is missing from remote server. (eg: because it was evicted by Redis)

    $remHash = $this->loadRemoteDataVersion($id);

    if ($remHash !== false && $this->getDataVersion($data) === $remHash) {
        $dataToSave = $this->remote->load($id);
    } else {
        $this->remote->save($data, $id, $tags, $specificLifetime);
        $this->saveRemoteDataVersion($data, $id, $tags, $specificLifetime);
    }

In order to avoid unnecessary writes on the remote server for both entries from point 1 and 2, it needs to read both keys content and TTL in order to create a condition that will cover all the cases. I find this solution overkill and I think it make more sense to force write both keys on remote server. The cache lock is a different component that could perform the above purpose of reducing the writes.

Important note: the remote cache consistency check should be performed in the load method

Follow the red arrows to see the failing scenario:

Manual testing scenarios (*)

Enable the L2 cache
Manually remove the data key for one of the cache entries from remote server, but keep the version cache key
Clear all the L2 cache entries
Check if the data cache key is saved to the remote server next time the code is executed

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

m2-assistant · 2022-11-03T17:07:38Z

Hi @georgebabarus. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

georgebabarus · 2022-11-04T06:17:17Z

@magento run all tests

magento-automated-testing · 2022-11-04T06:17:25Z