Manual Backup Temp (tmp) Files Not Removed
kbahey - October 7, 2008 - 01:50
| Project: | Backup and Migrate |
| Version: | 5.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
This log relates to the recent GoDaddy incident here http://drupal.org/node/313496.
They most probably killed the process before the backup completed, because it used excessive CPU time or memory, and therefore the temp files were left in the file system. The files were large and caused the account to go over its allotted disk space.
In order for this to not happen, despite the process being killed, two things need to happen:
1. Name the temp files with a more descriptive prefix, so it is obvious where they originate from. For example using tempnam("backup_migrate_") is better than the current "tmp_".
2. Have hook_cron() check for files with the above prefix daily, and clean out those that are older than a certain age (e.g. 6 hours).
#1
I agree with Khalid, Backup Migrate has managed to take down our über-beefy develoment server multiple times by filling up its hard drive. I also found another problem with tmp files that compounds this problem. If files are downloaded directly by the browser, the temp files are never deleted. Since this is what we do most often and our DB dumps are upwards of 800MB, this fills up our hard drives pretty quickly.
To test the direct download bug, just open up your tmp directory and do a direct to browser backup. 2 tmp files will be created and both left there after the file is created.
This patch does the following:
- Renames temporary files better.
- Deletes the temporary file that was *always* both empty and not deleted (tempnam() actually generates a file, not just a name).
- Adds a cron job to delete files that have been abandoned for more than 6 hours (hopefully no backup will take that long ;).
- Adds the missing hook_exit(), which is mentioned in backup_migrate_file_transfer() but doesn't actually exist.
Considering the amount of damage a full hard drive can cause, I'm marking this critical.
#2
Heh, wrong patch file ;)
#3
Thanks for the patch. I've been woefully absent from my queues due to work etc., but this is a high enough priority that I'll try and get it into all versions of the module this weekend.
Thanks again
Ronan
#4
Thanks, much appreciated ronan!
#5
I looked through the patch and it looks like it should take care of all kinds of temp file issues. I have applied the patch (or something like it) to all 4 branches (5.x-1.x, 5.x-2.x, 6.x-1.x, 6.x-2.x).
I will create new stable releases for the 1.x branches when I'm satisfied I didn't screw it up.
Thanks again for your help on this.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.