- First commit to issue fork.
- Status changed to Needs review
almost 2 years ago 12:23am 21 February 2023 - 🇦🇺Australia realityloop
@volegar I've cleaned this branch up to 10.1.x and recommitted the change with attribution to yourself and phenaproxima
- Status changed to Needs work
almost 2 years ago 10:43am 21 February 2023 - Issue was unassigned.
- 🇺🇸United States mglaman WI, USA
I think this should be using an SqlObjectStorage object for storing cache IDs per-request instead of request attributes. This avoids page_cache from manipulating the request. Just like \Drupal\Core\Path\CurrentPathStack
Copy of some work I'm adding to
protected function getCacheId(Request $request) { if ($this->cacheIds === NULL) { $this->cacheIds = new \SplObjectStorage(); } if (!isset($this->cacheIds[$request])) { $cid_parts = [ $request->getSchemeAndHttpHost() . $request->getRequestUri(), $request->getRequestFormat(NULL), ]; $this->cacheIds[$request] = implode(':', $cid_parts); } return $this->cacheIds[$request]; }
It doesn't matter what kind of request it is then. And may help solve issues if running Drupal in an event loop runtime.
- Merge request !5745Support multiple main requests with PageCache middleware → (Open) created by mglaman
- Status changed to Needs review
about 1 year ago 8:44pm 8 December 2023 - 🇺🇸United States mglaman WI, USA
I've created a new MR https://git.drupalcode.org/project/drupal/-/merge_requests/5745 that allows multiple main requests using the SplObjectStorage pattern, and a test. First commit showed test failure, second commit includes and shows the fix.
- 🇺🇸United States mglaman WI, USA
I tried to retitle to something more specific to the problem.
- Status changed to Needs work
12 months ago 4:56pm 5 January 2024 - 🇺🇸United States mglaman WI, USA
How should we handle the open thread. I don't agree with @catch that we need a comment about why we're supporting multiple cache IDs, when now PageCache is acting as a per-request reverse proxy instead of main-request-only reverse proxy.
- Status changed to Needs review
7 months ago 6:32pm 11 June 2024 - 🇺🇸United States smustgrave
Could a small comment be added? Then can remark it
- Status changed to Needs work
4 months ago 2:10pm 5 September 2024 - 🇨🇭Switzerland znerol
I'm not too happy keeping an unbound amount of state inside the page cache middleware. The original approach in #3029373: PageCache::getCacheId() sometimes uses different cache IDs during get and set, causing unnecessary cache misses for some requests → would be compatible with n main requests (look at the patches before comment
#13
.