search

itzg / docker-mc-backup

Provides a side-car container to backup itzg/minecraft-server world data
https://hub.docker.com/r/itzg/mc-backup
MIT License
298 stars 51 forks source link

rsync is not pruning all directories #176

Closed johnsturgeon closed 2 months ago

johnsturgeon commented 3 months ago

Found another issue with rsync method.

When pruning old directories, it does not work since the directories have 'files'.

The current prune command is this: find ${DEST_DIR} -type d -maxdepth 1 -mtime +1 -print -delete

The problem with rsync is that these are not just 'files' but rather directories, the -delete flag only works with files (it seems)

Here's the output from the docker console:

00c0cd8d962e:/backups# find . -type d -maxdepth 1 -mtime +1 -print
./world-20240416-072635
./world-20240416-072302
./world-20240416-072424
./world-20240416-072710
00c0cd8d962e:/backups# find . -type d -maxdepth 1 -mtime +1 -print -delete
./world-20240416-072635
find: ./world-20240416-072635: Directory not empty
./world-20240416-072302
find: ./world-20240416-072302: Directory not empty
./world-20240416-072424
find: ./world-20240416-072424: Directory not empty
./world-20240416-072710
find: ./world-20240416-072710: Directory not empty

I am going to write up a patch that uses -exec rm -rf {} \;

itzg commented 3 months ago

@toddejohnson can you help ponder this? The testing looked good in #165, so now I'm not sure what is different now.

itzg commented 3 months ago

I am going to write up a patch that uses -exec rm -rf {} \;

...that solution sounds good in any case

johnsturgeon commented 3 months ago

OK, circling back.. I don't know what's wrong actually. I can say with certainty that last night's backup did not prune my old directories, but it does look like rsync s prune does it correctly (my bad for mis-reading it from the original diagnosis) -- Maybe it needs to be -rf ??.

I'll let this run a few days and see if the issue reproduces itself, and maybe I'll add a bit more logging. In the mean time I'll hold off on a patch.

toddejohnson commented 3 months ago

Sure thing!

toddejohnson commented 3 months ago

The https://github.com/itzg/docker-mc-backup/blob/f3031a8c212af026cc604eb75751809a036727f4/scripts/opt/backup-loop.sh#L318 should be taking care of that with rm -r. The find is called from purge.

Debugging more tonight

@johnsturgeon Awesome find with the -a time issue! I was about to dig into that too.

toddejohnson commented 3 months ago

This was changed in #169

itzg commented 2 months ago

And #177 is now merged and built

johnsturgeon commented 2 months ago

This was changed in #169

Indeed! Thanks @toddejohnson . I must have had an older image, and when I was doing testing, pulled a new image and got your change.. I confused the daylights out of me since I DEFINITELY had the same issue ...

Nice to know I'm not going crazy