Opened 6 months ago
Closed 5 months ago
#61457 closed defect (bug) (fixed)
Delay automatic updates after installation.
Reported by: | costdev | Owned by: | costdev |
---|---|---|---|
Milestone: | 6.6 | Priority: | normal |
Severity: | normal | Version: | 6.6 |
Component: | Upgrade/Install | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
After installation, the user is directed to the Log In
page. This triggers the wp_schedule_update_checks()
function which is hooked to init
. As a result of the more robust maintenance mode added in [58128], the user may be presented with a maintenance mode screen just after installing WordPress.
To improve the user experience, there should be a delay of Core updates by 1 hour, plugin updates by 1.5 hours, and theme updates by 2 hours after installation.
This can be done by adding scheduled events for wp_version_check
, wp_update_plugins
and wp_update_themes
inside wp_install()
.
Change History (16)
This ticket was mentioned in PR #6828 on WordPress/wordpress-develop by @costdev.
6 months ago
#1
#2
@
6 months ago
- Keywords commit added
PR 6828 has been tested by @peterwilsoncc, @hellofromTonya and @nithi22, and approved by Peter as a Core Committer and Cron Component Maintainer, and Tonya as a Core Committer.
Marking for commit
.
#3
@
6 months ago
As an alternative; these offsets could be applied directly within wp_schedule_update_checks()
, such that the schedule is never enqueued at time()
but rather delayed from it.
Edit: This was raised and covered here: https://github.com/WordPress/wordpress-develop/pull/6828#issuecomment-2172701559
6 months ago
#4
My one question is why the scheduling is being modified in
wp_install()
rather than inwp_schedule_update_checks()
?
An additional but related reason is that other calls to
wp_schedule_update_checks()
expect those actions to be scheduled for an instant run when no existing event exists. Changing the scheduling inside the function would break expectations when it's unnecessary to do so.
Personally, I don't think there's a realistic expectation of that, and scheduling it at time()
is purely the default value that most choose to use as the starting point for cron tasks.
I don't feel strongly about this though, but I do feel it would be cleaner than the PR.
6 months ago
#5
@dd32 I see your point, and though the code changes would be cleaner if only modifying wp_schedule_update_checks()
directly, it's possible that this could have unintended consequences.
For example, code that clears the cron schedule for automatic updates, then runs wp_schedule_update_checks()
for a one-line call to start automatic updates checks immediately. Should further functionality and/or behaviour expect that to be immediate, we don't know the potential impact a delay for all use cases could have.
Given that this is possible, it's a risk I don't feel is worth taking to resolve the issue in the ticket, which is specifically in a post-installation context.
@peterwilsoncc commented on PR #6828:
6 months ago
#6
I chatted to Colin a bit further about when to delay/offset the cron jobs.
Given that the release is in beta, we think it might be best to reevaluate whether the offset can be added in the original location post release. That will allow for further work looking in to the situation described above and a considered decision about any implications.
6 months ago
#7
@dd32 Are you happy for us to commit this PR's changes for 6.6 Beta 3 and look at whether to change this to the wp_schedule_updates_checks()
function in a later release?
@hellofromTonya commented on PR #6828:
6 months ago
#8
Given that the release is in beta, we think it might be best to reevaluate whether the offset can be added in the original location post release. That will allow for further work looking in to the situation described above and a considered decision about any implications.
Good plan.
- Get the fix as is committed in time for the last scheduled beta release (which is in a few hours - 16 UTC).
- Then evaluate if there are "unintended consequences" for changing
wp_schedule_update_checks()
in the "post-installation context."
#11
@
5 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
@costdev Is there a reason why these cron events are scheduled so early, even before the notification emails are sent or cache is flushed?
We noticed that this change causes some WP-CLI tests to fail because the cron
option is added so early because of this, even before siteurl
is set.
IMHO this can be moved to right before the wp_install
hook fires.
This ticket was mentioned in Slack in #cli by swissspidy. View the logs.
5 months ago
#13
@
5 months ago
@swissspidy They were added to their current location in the function only because it's the first stage after the database tables they need have been created. At the moment, I don't know if moving it further down will cause any issues (I suspect not), so any change would, as always, need to be re-tested. I'm limited on bandwidth at the moment, so I'm not sure I'll be able to carry out this follow-up work and testing, but I'll join the effort if I'm able to.
#14
@
5 months ago
@swissspidy Can you detail the failure (expected, previous actual and new actual) and the specifics of why the presence of a cron
option causes, and should cause, a failure in the tests?
I trust that it's a problem with this change rather than with how the tests were written, so the request for detail is for posterity to confirm and document this for our future selves.
#15
@
5 months ago
I primarily noticed it because the option ID changed, as the tests assume siteurl
has the ID 1. While we can probably just adjust our tests, I am a bit concerned about other possible consequences of this change.
Other tools/plugins could rely on siteurl
having ID 1, and there is a potential risk because of all the cron-related filters running before the site is actually installed (no options or roles populated, etc.) and before any upgrade routines run. So if you have anything that hooks into those, they might not work correctly because the site isn't fully installed yet.
I understand the need to add these cron changes earlier, but there is no reason for them to be _that_ early.
While it still might be OK, I wanna flag this nonetheless.
Apologies for not raising this earlier, but I was mostly out for the past few weeks.
Generally speaking, it would be good to think more of WP-CLI when making changes to crucial parts such as the installation process. Maybe with core running WP-CLI tests or so.
Note:
The changes in this PR cannot be tested in a
wordpress-develop
checkout because automatic updates do not run in version-controlled systems. Download 6.6 Beta X, then paste these changes in place before attempting to install the site.Alternatively, if you have permissions for this repo, download the build artifact from the bottom of this page.
---
After installation, the user is directed to the
Log In
page. This triggers thewp_schedule_update_checks()
function which is hooked toinit
. As a result, the user may be presented with a maintenance mode screen just after installing WordPress.To improve the user experience, this delays Core updates by 1 hour, plugin updates by 1.5 hours, and theme updates by 2 hours.