Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#25655 closed defect (bug) (fixed)

Disable maintenance mode as early as possible

Reported by: nacin's profile nacin Owned by: nacin's profile nacin
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description

To limit the amount of time we spend in maintenance mode, let's move it up a few lines, to before the HTTP request to automatically trigger a DB upgrade (which has a timeout of 60), and before the deletion of old files.

Normally, a DB upgrade would be triggered by anyone visiting wp-admin. We do it ourselves, but I don't think we need to keep the site down in maintenance mode during it. I'm not sure but I don't think it is easy for a race condition to occur to trigger multiple upgrades at once. That said, with possible major schema changes in the works in the future, we may want to keep the schema update behind maintance mode. But since this code resides in the wp-admin/includes/update-core.php file that gets copied over, that can be finagled with on a case-by-case basis.

In order to keep sites in maintenance mode for as little time as possible (potentially only a few seconds), we should exit maintenance mode before the DB upgrade for 3.7. Or, at worst, we should make sure old files don't delay the maintenance mode.

Attachments (3)

25655.diff (1.1 KB) - added by DaveAl 11 years ago.
25655.2.diff (1.3 KB) - added by dd32 11 years ago.
25655.3.diff (1.3 KB) - added by dd32 11 years ago.

Download all attachments as: .zip

Change History (11)

#1 @DaveAl
11 years ago

  • Keywords has-patch added

@DaveAl
11 years ago

@dd32
11 years ago

#2 @dd32
11 years ago

25655.diff moves the disabling to above the DB upgrade, but after the $_old_files check.

25655.2.diff moves the disabling to directly after the core copy process, but BEFORE, languages, plugins, themes, $_old_files, and the DB upgrade.
If our intention for maintenance mode is for no visitor to get a PHP error page instead, that would be the absolute minimum time possible for maintenance mode, which feels like the correct thing to do here.

#3 @nacin
11 years ago

I think we can keep maintenance mode until we're done with languages, plugins, and themes. For an automatic background update done with a partial build, all of that will be skipped over anyway. So moving it up above DB upgrade and $_old_files seems OK for 3.7.

#4 follow-up: @nacin
11 years ago

  • Keywords commit added

I forget that only development versions get updates to wp-content. For stable, we are simply installing something new, rather than updating a theme possibly in use. And languages don't matter, obviously. OK, let's do 25655.2.diff.

#5 in reply to: ↑ 4 ; follow-up: @dd32
11 years ago

Replying to nacin:

I forget that only development versions get updates to wp-content. For stable, we are simply installing something new, rather than updating a theme possibly in use. And languages don't matter, obviously. OK, let's do 25655.2.diff.

That was exactly my thinking behind the patch, Only languages could possibly be affected - which brings me to.. Languages.
If we have any locale-specific builds with a locale.php file, we should probably stay in maintenance mode while we copy that over too.. perhaps we should slip it in after the language copy function instead?

#6 in reply to: ↑ 5 @nacin
11 years ago

Replying to dd32:

Replying to nacin:

I forget that only development versions get updates to wp-content. For stable, we are simply installing something new, rather than updating a theme possibly in use. And languages don't matter, obviously. OK, let's do 25655.2.diff.

That was exactly my thinking behind the patch, Only languages could possibly be affected - which brings me to.. Languages.
If we have any locale-specific builds with a locale.php file, we should probably stay in maintenance mode while we copy that over too.. perhaps we should slip it in after the language copy function instead?

Fine with that. At worst, normally, we end up with a single extra is_dir() check for packages that don't have languages.

@dd32
11 years ago

#7 @dd32
11 years ago

Did some extra timing on the upgrade code.

There's only 4 significant parts of the code

  • Downloading the package (~15 seconds for me)
  • Unziping the package (~5 seconds for me)
  • Updating the files (~5 seconds for me)
  • Performing the loopback HTTP request to upgrade the DB (~8 seconds for me)

Everything outside of those steps is tiny and counted in a few ms here, a few ms there.

25655.3.diff moves the disabling to the most logical place, after core and language files are moved, before extra themes and old files are dealt with, as nacin says above for most people it'll only be a few extra milliseconds in is_dir(), for others, it may be another second while it updates the language files.

Last edited 11 years ago by dd32 (previous) (diff)

#8 @nacin
11 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25866:

Updates: Disable maintenance mode once we've finished copying files, to minimize disruption.

props dd32.
fixes #25655.

Note: See TracTickets for help on using tickets.