Opened 11 years ago
Closed 11 years ago
#25655 closed defect (bug) (fixed)
Disable maintenance mode as early as possible
Reported by: | nacin | Owned by: | 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)
Change History (11)
#3
@
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:
↓ 5
@
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:
↓ 6
@
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
@
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.
#7
@
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.
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.