Make WordPress Core

Opened 6 months ago

Closed 5 months ago

#61457 closed defect (bug) (fixed)

Delay automatic updates after installation.

Reported by: costdev's profile costdev Owned by: costdev's profile 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

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 the wp_schedule_update_checks() function which is hooked to init. 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.

#2 @costdev
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 @dd32
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

Last edited 6 months ago by dd32 (previous) (diff)

@dd32 commented on PR #6828:


6 months ago
#4

My one question is why the scheduling is being modified in wp_install() rather than in wp_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.

@costdev commented on PR #6828:


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.

@costdev commented on PR #6828:


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."

#9 @costdev
6 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58435:

Upgrade/Install: Delay automatic updates after installation.

After installation, the user is directed to the Log In page. This triggers the wp_schedule_update_checks() function which is hooked to init and schedules updates to run immediately if no other events exist. As a result of more robust use of maintenance mode for automatic updates added in [58128], the user may be presented with a maintenance mode screen just after installing WordPress.

To improve the user experience, this schedules core updates for 1 hour, plugin updates for 1.5 hours, and theme updates for 2 hours after installation.

Follow-up to [58128], [58139], [58308], [58309].

Props afragen, hellofromTonya, peterwilsoncc, nithi22, dd32.
Fixes #61457. See #58281, #61391.

@costdev commented on PR #6828:


6 months ago
#10

Committed in r58435.

#11 @swissspidy
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 @costdev
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.

Last edited 5 months ago by costdev (previous) (diff)

#14 @costdev
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.

Last edited 5 months ago by costdev (previous) (diff)

#15 @swissspidy
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.

#16 @swissspidy
5 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

After improving the WP-CLI tests I couldn't find other issues, so I am closing this again for now, though I wanna stress again that this should be kept an eye on.

Note: See TracTickets for help on using tickets.