Bug when restoring svg files from backup

Contribute Bug

AI-generated summary

A bug was reported when restoring SVG files from a backup. The issue occurs because SiteIconManager.ensure_optimized! runs before the images have been extracted, and it expects the files to be present in the default directory. This causes the restore process to fail. The bug was investigated and a minimal reproduction case was provided. The issue has been fixed in a recent commit gerhard. The fix addresses the incorrect ordering of db:migrate and image extraction, and also ensures that SiteIconManager.ensure_optimized! does not run before the images are available.

1

When trying to restore a backup (to multisite, but that does not seem to matter, see below) the following happens

Migrating the database...
EXCEPTION: lib/discourse.rb:57:in `exec': Failed to migrate database.
rake aborted!
Errno::ENOENT: No such file or directory @ rb_sysopen - /var/www/discourse/public/uploads/default/original/2X/7/7be997f9f48c034ddf5d4eb95d2ea7416f010241.svg
/var/www/discourse/app/models/optimized_image.rb:87:in `block in create_for'
/var/www/discourse/app/models/optimized_image.rb:24:in `block (2 levels) in lock'
/var/www/discourse/lib/distributed_mutex.rb:33:in `block in synchronize'
/var/www/discourse/lib/distributed_mutex.rb:29:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:29:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:14:in `synchronize'
/var/www/discourse/app/models/optimized_image.rb:23:in `block in lock'
/var/www/discourse/lib/distributed_mutex.rb:33:in `block in synchronize'
/var/www/discourse/lib/distributed_mutex.rb:29:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:29:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:14:in `synchronize'
/var/www/discourse/app/models/optimized_image.rb:22:in `lock'
/var/www/discourse/app/models/optimized_image.rb:59:in `create_for'
/var/www/discourse/lib/site_icon_manager.rb:28:in `block in ensure_optimized!'
/var/www/discourse/lib/site_icon_manager.rb:24:in `each'
/var/www/discourse/lib/site_icon_manager.rb:24:in `ensure_optimized!'
/var/www/discourse/lib/tasks/db.rake:83:in `block in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.6.0/gems/rake-13.0.1/exe/rake:27:in `<top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.6.0/bin/ruby_executable_hooks:24:in `eval'
/var/www/discourse/vendor/bundle/ruby/2.6.0/bin/ruby_executable_hooks:24:in `<main>'
Tasks: TOP => db:migrate

At first I thought this was purely a multisite issue but it is failing on a non-multisite as well.

Funny thing is, it was not even extracting the uploads (EDIT: at that point in time). That specific upload is included in the archive though.

tar tvf public/backups/default/redacted-forum-2020-03-02-165725-v20190130013015.tar.gz |grep 7be997f9f48c034ddf5d4eb95d2ea7416f010241
-rw-r--r-- daemon/daemon     3074 2019-05-27 08:21 uploads/default/original/2X/7/7be997f9f48c034ddf5d4eb95d2ea7416f010241.svg

Original database dump version v20190130013015

I then extracted the uploads manually (into default) and then ran the restore again, which succeeded.

And then I saw it… the uploads are extracted AFTER db:migrate, but db:migrate is running SiteIconManager.ensure_optimized! requiring the files to be present already…

So there are two issues:

  • SiteIconManager.ensure_optimized! runs before the images have been extracted; (actually it is running twice - it is running another time after the uploads have been extracted)
  • it is expecting the files in default because the remap hasn’t been executed yet.

In this specific case this is failing hard because the code for an svg upload takes a different code path. For other uploads it seems to be failing silently.

BTW looking at the code it seems like this could be circumvented by using SKIP_POST_DEPLOYMENT_MIGRATIONS=1 since that skips the image optimization during db:migrate but that didn’t work when I tried it.

6 Likes
2

Who should look at this one @sam?

1 Like
3

It seems pretty straight forward:

And

Last person to play around with this flow was @gerhard

@RGJ if you would like to speed up diagnostics here, maybe make a blank db with a single SVG the fails restore, that will make it way easier for us to test.

7 Likes
5

I sent you a PM with an as-lean-as-possible backup file that does not restore.

It requires a “small” svg file on favicon or manifest for the bug to trigger.

BTW I just found out that SKIP_POST_DEPLOYMENT_MIGRATIONS is explicitly reset so that is why that did not work as a workaround.

5 Likes
6

Thanks for reporting the issue and providing an example file. Is has been fixed.

https://github.com/discourse/discourse/commit/8fa8bab9ff361bfa36592a6e32cfb96a4fc38d06

6 Likes