#456824: drupal_lookup_path() speedup - cache system paths per page. introduced per page cached path aliases. However, there are two issues with the current implementation:

  • AJAX autocomplete handlers create cache records containing only one entry (the AJAX handler's URL itself)
  • invalid page requests create cache records.

These records may fill up the cache table with thousands of invalid entries, eventually taking up all available database space.

Therefore, the code should obey the following two conditions:

  • Only write a cache record if the path alias map contains more than one entry, which rules out AJAX handlers. And caching just one path won't result in any performance gain anyway (one cache lookup vs. one direct lookup).
  • Never cache the path alias map on error pages, i.e. anything with a HTTP 4xx code.

Also tagging with Pressflow, which contains a backport of this feature for D6.

CommentFileSizeAuthor
#3 core-cache_path_cleanup-902736-3.patch1.2 KBAkaoni
#6 core-cache_path_cleanup-test-902736-3.patch1.16 KBdawehner

Comments

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Performance

Subscribing.

First point seems like a no brainer.

Second point there are all kinds of per path caches and processing that happens on 404 and 403 pages, for preventing caching the should be a standard pattern if we wanted to do that.

Akaoni’s picture

Akaoni commented
Version: 8.x-dev » 7.x-dev
Status: Needs work » Active
Issue tags: -Needs backport to D7

It also seems that expired cache_path entries aren't cleaned up which compounds the problem.
Edit: Not relevant to this issue.

Akaoni’s picture

Akaoni commented
Status: Active » Needs review
StatusFileSize
new core-cache_path_cleanup-902736-3.patch1.2 KB

Quick cache_path cleanup patch.
Edit: Not relevant to this issue.

berenddeboer’s picture

berenddeboer commented
Priority: Normal » Major
Status: Active » Needs review

This looks like an important bug, and explains why my database are filling up!

Akaoni’s picture

Akaoni commented

@berenddeboer: Which issue is an important bug?
I think I might have muddied the waters adding the fact that cache_path isn't cleaned up here.
My bad. :(

Worth splitting this into two issues?

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new core-cache_path_cleanup-test-902736-3.patch1.16 KB

Here is a test patch for the patch above. This should probably fail.

dawehner’s picture

dawehner
Primary language German
commented

Expect this patch to fail.

Status: Needs review » Needs work

The last submitted patch, core-cache_path_cleanup-test-902736-3.patch, failed testing.

Akaoni’s picture

Akaoni commented
Version: 7.x-dev » 8.x-dev
Priority: Major » Normal
Issue tags: +Needs backport to D7

Moved the cache_path not being cleaned up issue to it's own thread:
http://drupal.org/node/1259096
Sorry again for hijacking this one.

Moved this issue to 8.x-dev and tagged for backport.

Status: Needs review » Needs work

Correcting issue status, see #2125263: Issue status not taken from last comment during D7 migration.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.