#54668 closed enhancement (fixed)
Non blocking wp-cron.php with LSAPI
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Cron API | Keywords: | has-patch add-to-field-guide |
Focuses: | performance | Cc: |
Description
As a follow-up to that change [44488] (Make wp-cron.php non-blocking where possible)
Currently, it's only done with FastCGI, but it's also possible with LiteSpeed LSAPI using litespeed_finish_request()
.
fastcgi_finish_request()
was an alias for a short time in LSAPI but it's not anymore, so wp-cron.php is blocking with LSAPI.
Could we consider implementing it in wp-cron.php?
Others already done that: https://github.com/symfony/symfony/pull/42332
and I don't see any issues using it in wp-cron.php.
Change History (11)
#2
@
18 months ago
Thanks @johnbillion, will do a PR soon (not familiar with the testing steps however).
But before, it leads me to another point :
With #47396 and the following changesets [45560] and [46458]
no cache headers are sent for these "early finished requests"
if ( ! headers_sent() ) {
header( 'Expires: Wed, 11 Jan 1984 05:00:00 GMT' );
header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
}
But it was stated that calls to wp-cron.php are not intended to be cached, so why sending these headers only on that non blocking case ?
wp-cron.php can be cached if hosted on incompatible servers.
Do I miss something @peterwilsoncc?
I get that
the fastcgi_finish_request() call prevents the setting of further HTTP headers by caching plugins subsequently loaded.
but no non caching headers are sent afterwards by default.
This ticket was mentioned in PR #2155 on WordPress/wordpress-develop by Maxime-J.
17 months ago
#3
- Keywords has-patch added; needs-patch removed
LSAPI support added for a non-blocking wp-cron.php
Headers intended to prevent caching moved outside of the non-blocking scenario, so that they are also sent if non-blocking isn't supported
Trac ticket: https://core.trac.wordpress.org/ticket/54668
#4
@
11 months ago
Hi @johnbillion @peterwilsoncc
Sorry if I am bothering you, but any news on that being considered?
Support for non blocking in LSAPI set appart, the fact that headers to prevent caching are only sent with FastCGI looks like a problem to me. Do I miss something?
#5
@
11 months ago
- Milestone changed from Awaiting Review to 6.1
@maximej I've just updated your pull request to resolve some merge conflicts.
I've put this on the milestone for the next major version of WordPress.
Do you know which version of the LSAPI removed the fastcgi_finish_request()
alias? It's not in the release log and I am wondering if there was a bug with ending requests early that the developers were avoiding.
#6
@
11 months ago
I get that (
fastcgi_finish_request
prevents setting further headers but) no non caching headers are sent afterwards by default.
Sorry, I missed this question earlier today.
The idea behind placing the headers inside the fastcgi_finish_request
was to ensure that any full page caching plugins retained control of how to manage the wp-cron cache. Most of them do a check for DOING_CRON === true
and send out their own headers.
To avoid repeating code multiple times, I think the change you have in your pull request is for the best. Caching plugins calling header()
with expiry headers later in the process will replace the existing headers.
#7
@
11 months ago
- Owner set to peterwilsoncc
- Resolution set to fixed
- Status changed from new to closed
In 53653:
peterwilsoncc commented on PR #2155:
11 months ago
#8
Merged in 1ff92618f22b92683ca85bff30f368f76d86d9c5 -- canonically https://core.trac.wordpress.org/changeset/53653
#9
@
11 months ago
Thanks for the change and the explanation @peterwilsoncc !
Concerning the alias removal, here is the according commit
https://github.com/php/php-src/commit/ccf051c317e606c2a4d9099c6e79a5c42bfdb298
just a few days after they released litespeed_finish_request()
Apparently, there was indeed a bug, fixed in that commit. That may be why they removed the alias, as a precaution.
It was stated "for now" in the commit, but the alias actually never went back in.
Welcome @maximej and thanks for the ticket.
This sounds like it makes sense. PRs are welcome against https://github.com/WordPress/wordpress-develop.
Could you provide the testing steps necessary please?