I have Backup and Migrate running on a number of websites. The schedules I have set up are configured with the smart delete option, and a range of backup frequencies. Recently when checking the backup directories for the different sites I noticed that smart delete was deleting to many files. It appears to only be saving files of the most frequent backup period. e.g. if set to backup daily it will save the daily backups but not the weekly ones, if set to backup hourly it will save the hourly ones but not the daily or weekly ones. From my understanding of the code the issue appears to be with the time reference being now (i.e. a sliding point through time) and the backups being a fixed time. Inevitably backups find themselves in the gap between being saved by the hourly period and being saved by the daily period and get deleted. This patch takes a different approach and uses the first backup as the reference time for saving future backups. This patch is currently running successfully on three production sites.

P.S. I have no idea how to propose that this patch be added to the main branch, so any advice on that matter would be appreciated.

CommentFileSizeAuthor
#9 backup_migrate-smart-delete-not-saving-2606586-9.patch3.59 KBDarren Oh
#5 smart_delete_not_saving-2606586-5.patch3.59 KBDarren Oh
#4 Smart_Delete_not_saving_files_for_all_periods___2606586____Drupal_org.png111.64 KBjoelpittet
#2 smart_delete_losing_files.patch3.21 KBdmargetts
smart_delete_losing_files.patch3.32 KBdmargetts
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dmargetts created an issue. See original summary.

dmargetts’s picture

dmargetts CreditAttribution: dmargetts commented
FileSize
smart_delete_losing_files.patch3.21 KB
Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh at Software Paradigms International (SPI) for Estée Lauder Companies commented
Version: 7.x-3.1 » 7.x-3.x-dev
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue summary: View changes
Status: Needs review » Needs work
FileSize
Smart_Delete_not_saving_files_for_all_periods___2606586____Drupal_org.png111.64 KB

@dmargetts could you recreate that patch with 2 space indents so dreditor doesn't have a fit?
https://www.drupal.org/coding-standards#indenting

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh at Software Paradigms International (SPI) for Estée Lauder Companies commented
Title: Smart Delete not saving files for all periods » Fix smart delete not saving files for all periods.
Priority: Normal » Major
Status: Needs work » Needs review
FileSize
smart_delete_not_saving-2606586-5.patch3.59 KB

I have reformatted the patch.

bjcooper’s picture

bjcooper CreditAttribution: bjcooper commented

The patch in #5 seems to work for me! I hope we can get this reviewed and committed—finding out there weren't any daily backups was an unpleasant surprise.

dddbbb’s picture

dddbbb CreditAttribution: dddbbb as a volunteer commented
Status: Needs review » Needs work

The patch in #5 isn't working in my test where we appear to now be retaining hourly backups beyond 24 hours ago.

couturier’s picture

couturier CreditAttribution: couturier as a volunteer commented

I'm not sure what to tell you as far as getting this patch committed. You might try the new 7.x-3.2 version that was released September 27, 2017, to see if it affects the issue. The previous maintainers for Backup and Migrate have ceased supporting this module, and a new maintainer is focusing efforts on the port to D8, so resources for fixing the 7.x branch are limited. You can expect a stable release for D8 soon if you're migrating sites.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh as a volunteer commented
Title: Fix smart delete not saving files for all periods. » Fix smart delete not saving files for all periods
Status: Needs work » Needs review
FileSize
backup_migrate-smart-delete-not-saving-2606586-9.patch3.59 KB

Re-rolled patch.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

Thanks for the reroll, Darren.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

Patch #5 was tested on PHP 7.2, but patch #9 is on PHP 7.1. Is this intentional?

Also, I've been using patch #5 for 18 months, and it seems to keep the daily and weekly backups properly in addition to the hourly, whereas on a site that I don't use it, I still have the issue. So I'd say I consider the patch to be tested.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh as a volunteer commented

The PHP version is not related to a patch’s failure to apply. If it fails to apply, no tests are run. Patch #9 is just a re-roll of patch #5 to get it apply to the current code. It had not been committed because no one updated the status of this issue. If patch #5 has been working for you, please update the status of this issue to Reviewed & tested by the community.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #3020775: Plan for Backup and Migrate 7.x-3.7

Thanks for the updates.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

bmango’s picture

bmango CreditAttribution: bmango commented

Is there any possiblity there could be a new release of a production version for Backup and Migrate 7? Just because the current production version 7.x-3.6 does not include the fix for Smart Delete. I think quite a few sites are affected by this, mine included. I was hoping to retrieve data from a few months ago but realised it was not possible. When people see the new production version they can automatically update their version whereas they won't know about the change if it is just in dev. :)

bmango’s picture

bmango CreditAttribution: bmango commented

Thank you for commiting the patch to dev btw!