Add caching to wp_old_slug_redirect

[spacedmonkey]

The wp_old_slug_redirect function is called on every page. It perform a raw sql query everytime it is run. The result of that query should be cached for performance reasons.

[dd32]

The wp_old_slug_redirect() function only runs when the current pageload is marked as a 404, based on the is_404() check at the start of the function.

I don't think caching is the answer here, rather the custom code you're using to make a "fake" page needs to flag the page as not-a-404 to WordPress.

[spacedmonkey]

Sorry @dd32 I am not sure what you mean by "fake" page. The patch just caches the response from sql. That is then invalided when the clean_post_cache function is called and the last_changed value changes. I know WordPress VIP are doing something similar with caching the result of this function. Still not sure why this needs to call sql on every time. You could easily have a popular url that is 404ing as you remove the post. This just increase traffic to sql unnecessarily.

[dd32]

Replying to spacedmonkey:

Sorry @dd32 I am not sure what you mean by "fake" page.

Sorry, it was an assumption on my part - When someone says that a function that has a is_404() check in it is running every page, it generally means they're running a plugin which injects a "fake" page (one that doesn't actually result in a proper WP_Query object being located).

What I mean, is that the only time that query should ever be run, is in the case where a singular page (a non-hierarchical one at that) is requested and cannot be found.
Caching the function in that case doesn't seem to be the best location for it - it seems like the redirect should be cached instead to me.

[spacedmonkey]

I miss spoke when I said it was on every page load. I meant it hooked on template redirect on every page load.

So a little back story on this. I run a large high traffic multisite. Lots of editorial teams. I also use object caching to help with performance. While looking into sql log I found a high number of calls to this sql query. We don't cache 404s in our cdn, so this function is results in a high amount of sql traffic.

I am not sure it is possible to cache the redirect it's self. After getting the post ID it calls get permalink. The value of the permalink can be dynamic, as either it is filtered or the home_url changes, (ssl or domain mapping). I think that just save the result of sql is the best option, as it lets those filters run and doesn't break backwards compatibility.

Not working yet
the new tests are not work

Trac ticket: https://core.trac.wordpress.org/ticket/36723

There are php lint errors in this PR. Can you run composer lint / composer format locally to fix these.

[spacedmonkey]

Refreshed my patch as a PR ​#2448.

Trac ticket: https://core.trac.wordpress.org/ticket/36723

I have own version of this patch at - ​https://github.com/WordPress/wordpress-develop/pull/2448

This includes unit tests.

CC @pbearne

[mukesh27]

The PR needs to update against the trunk.

@spacedmonkey Is this ticket is in your to-do list for the upcoming release?

[spacedmonkey]

@mukesh27 Yes it is. Why do you want to take over it?

[mukesh27]

Thanks for the quick reply and set the milestone. I want to know if it lands on the upcoming release.

Happy to help in any way I can also test the PR when it is ready.

[spacedmonkey]

In 53549:

Posts, Post Types: Add caching to _find_post_by_old_slug and _find_post_by_old_date functions.

Cache result of database queries in _find_post_by_old_slug and _find_post_by_old_date functions. This means that repeated requests to a url where the page is not found, will result in hitting a cache for sites running persistent object caching.

Props Spacedmonkey, dd32, mukesh27, pbearne, flixos90.
Fixes #36723.

Committed.

Committed.

[desrosj]

There are a lot of caching related changes in 6.1 and a dev note should be written collecting them all. Marking this for inclusion.