[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