[wp-trac] [WordPress Trac] #51857: Add rollback for failed plugin/theme updates
WordPress Trac
noreply at wordpress.org
Thu Jan 21 01:58:35 UTC 2021
#51857: Add rollback for failed plugin/theme updates
-------------------------------------------------+-------------------------
Reporter: pbiron | Owner: pbiron
Type: enhancement | Status: assigned
Priority: normal | Milestone: 5.7
Component: Upgrade/Install | Version:
Severity: normal | Resolution:
Keywords: has-patch dev-feedback needs- | Focuses:
testing early |
-------------------------------------------------+-------------------------
Comment (by dd32):
Replying to [comment:49 SergeyBiryukov]:
> It looks like there are some concerns about performance implications
[...] could we rename the plugin directory to something reasonably unique
like `plugin-slug.__rollback__`, and then delete it on successful update,
or rename back on failure?
This looks like the best option from a filesystem-io perspective to me.
Replying to [comment:52 afragen]:
> Unfortunately `rename()` can be flaky/unreliable and fallback from
`rename()` involves file copy which is the likely cause of the failure in
the first place.
I'm not sure what the flaky/unreliable experience is here, but I feel like
creating a ZIP backup is solving a problem that doesn't actually exist
while adding significant CPU/IO into the process at the same time that's
not needed.
Looking at the filesystem handlers `move()`, it looks like the Direct IO
one has some behaviour to use a copy, which _probably_ isn't needed and is
probably not useful, [13001] added the usage of `rename()` into it. A lot
has changed in the last decade host IO wise.
Also of note, is that the FTP variants don't check the overwrite
functionality.. but that's mostly irrelevant here.
I would probably suggest just cleaning up those `move()` methods to be
consistent and not fallback on a `copy()` if it's not actually performing
a `move()`/removing the source directory.
IMHO if `move()` fails, either the delete of the existing directory will
also fail, or rollback should just be skipped for those edge-cases.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/51857#comment:55>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list