Upload remapping is a bit too enthousiastic

RGJ · 5월 28, 2019, 3:44오후

When a backup is restored to a database with a different name, there is a neat little piece of code in restorer.rb that attempts to fix this.

github.com/discourse/discourse

lib/backup_restore/restorer.rb

main


      
          # frozen_string_literal: true
          
          require "colored2"
          
          module BackupRestore
            RestoreDisabledError = Class.new(RuntimeError)
            FilenameMissingError = Class.new(RuntimeError)
          
            class Restorer
              delegate :log, to: :@logger, private: true
          
              attr_reader :success
          
              def initialize(
                user_id:,
                filename:,
                factory:,
                disable_emails: true,
                location: nil,
                interactive: false
              )
                @user_id = user_id
                @filename = filename
                @factory = factory
                @logger = factory.logger
                @disable_emails = disable_emails
                @interactive = interactive
          
                ensure_restore_is_enabled
                ensure_we_have_a_user
                ensure_we_have_a_filename
          
                @success = false
                @current_db = RailsMultisite::ConnectionManagement.current_db
          
                @system = factory.create_system_interface
                @backup_file_handler = factory.create_backup_file_handler(@filename, @current_db, location)
                @database_restorer = factory.create_database_restorer(@current_db)
                @uploads_restorer = factory.create_uploads_restorer
              end
          
              def run
                log "[STARTED]"
                log "'#{@user_info[:username]}' has started the restore!"
          
                # FIXME not atomic!
                ensure_no_operation_is_running
                @system.mark_restore_as_running
          
                @system.listen_for_shutdown_signal
          
                @filename, @tmp_directory, db_dump_path = @backup_file_handler.decompress
                validate_backup_metadata
          
                @system.enable_readonly_mode
                @system.pause_sidekiq("restore")
                @system.wait_for_sidekiq
                @system.flush_redis
                @system.clear_sidekiq_queues
          
                @database_restorer.restore(db_dump_path, @interactive)
          
                reload_site_settings
                run_seed_fu
          
                @system.disable_readonly_mode
          
                clear_category_cache
                clear_stats
                reload_translations
          
                restore_uploads
          
                clear_emoji_cache
                clear_theme_cache
          
                after_restore_hook
              rescue Compression::Strategy::ExtractFailed
                log "ERROR: The uncompressed file is too big. Consider increasing the hidden " \
                      '"decompressed_backup_max_file_size_mb" setting.'
                @database_restorer.rollback
              rescue SystemExit
                log "Restore process was cancelled!"
                @database_restorer.rollback
              rescue => ex
                log "EXCEPTION: " + ex.message
                log ex.backtrace.join("\n")
                @database_restorer.rollback
              else
                @success = true
              ensure
                clean_up
                notify_user
                log "Finished!"
          
                @success ? log("[SUCCESS]") : log("[FAILED]")
              end
          
              protected
          
              def ensure_restore_is_enabled
                return if Rails.env.development? || SiteSetting.allow_restore?
                raise BackupRestore::RestoreDisabledError
              end
          
              def ensure_no_operation_is_running
                raise BackupRestore::OperationRunningError if BackupRestore.is_operation_running?
              end
          
              def ensure_we_have_a_user
                user = User.find_by(id: @user_id)
                raise Discourse::InvalidParameters.new(:user_id) if user.blank?
          
                # keep some user data around to check them against the newly restored database
                @user_info = { id: user.id, username: user.username, email: user.email }
              end
          
              def ensure_we_have_a_filename
                raise BackupRestore::FilenameMissingError if @filename.nil?
              end
          
              def validate_backup_metadata
                @factory.create_meta_data_handler(@filename, @tmp_directory).validate
              end
          
              def reload_site_settings
                log "Reloading site settings..."
                SiteSetting.refresh!
          
                DiscourseEvent.trigger(:site_settings_restored)
          
                if @disable_emails && SiteSetting.disable_emails == "no"
                  log "Disabling outgoing emails for non-staff users..."
                  user = User.find_by_email(@user_info[:email]) || Discourse.system_user
                  SiteSetting.set_and_log(:disable_emails, "non-staff", user)
                end
              end
          
              def run_seed_fu
                log "Running seed fu..."
          
                begin
                  Discourse::Application.load_tasks
                  Rake::Task["db:seed"].invoke
                rescue => ex
                  log "Failed to run seed fu", ex
                end
              end
          
              def clear_category_cache
                log "Clearing category cache..."
                Category.reset_topic_ids_cache
                Category.clear_subcategory_ids
              end
          
              def clear_emoji_cache
                log "Clearing emoji cache..."
                Emoji.clear_cache
              rescue => ex
                log "Something went wrong while clearing emoji cache.", ex
              end
          
              def reload_translations
                log "Reloading translations..."
                TranslationOverride.reload_all_overrides!
              end
          
              def restore_uploads
                if @interactive
                  puts ""
                  puts "Attention! Pausing restore before uploads.".red.bold
                  puts "You can work on the restored database in a separate Rails console."
                  puts ""
                  puts "Press any key to continue with the restore.".bold
                  puts ""
                  STDIN.getch
                end
          
                @uploads_restorer.restore(@tmp_directory)
              end
          
              def notify_user
                return if @success && @user_id == Discourse::SYSTEM_USER_ID
          
                if user = User.find_by_email(@user_info[:email])
                  log "Notifying '#{user.username}' of the end of the restore..."
                  status = @success ? :restore_succeeded : :restore_failed
          
                  logs = Discourse::Utils.logs_markdown(@logger.logs, user: user)
                  SystemMessage.create_from_system_user(user, status, logs: logs)
                else
                  log "Could not send notification to '#{@user_info[:username]}' " \
                        "(#{@user_info[:email]}), because the user does not exist."
                end
              rescue => ex
                log "Something went wrong while notifying user.", ex
              end
          
              def clean_up
                log "Cleaning stuff up..."
                @database_restorer.clean_up
                @backup_file_handler.clean_up
                @system.unpause_sidekiq
                @system.disable_readonly_mode if Discourse.readonly_mode?
                @system.mark_restore_as_not_running
              end
          
              def clear_theme_cache
                log "Clear theme cache"
                ThemeField.force_recompilation!
                Theme.expire_site_cache!
                Stylesheet::Manager.cache.clear
              rescue => ex
                log "Something went wrong while clearing theme cache.", ex
              end
          
              def clear_stats
                Discourse.stats.remove("missing_s3_uploads")
              end
          
              def after_restore_hook
                log "Executing the after_restore_hook..."
                DiscourseEvent.trigger(:restore_complete)
              end
            end
          end

This file has been truncated. show original

However, this renames ALL upload filenames, not just the ones that were extracted from the archive. If the database name occurs in an S3 path (which obviously does not change), it changes it as well, causing all images to fail loading.

codinghorror · 5월 29, 2019, 12:44오전

What do you think here @tgxworld?

supermathie · 5월 29, 2019, 4:15오후

Would changing this to include the trailing slash have helped?

DbHelper.remap("uploads/#{previous_db_name}/", "uploads/#{current_db_name}/")

That feels more correct in any case.

RGJ · 5월 29, 2019, 4:59오후

No - I’m talking about filenames like //discourse-cloud-file-uploads.s3.dualstack.us-west-2.amazonaws.com/standard11/uploads/DBNAME/original/1X/abcd.....jpg

So I think we’d need to exclude uploads starting with // or something like that.

tgxworld · 5월 30, 2019, 1:34오전

We’re experiencing the pain of this quite abit while migrating local uploads to S3. Unfortunately, no amount of hacky remaps can fix this properly. What we’re currently working on at the moment is to remove/disallow upload links in Post#raw. The short upload url, ![test](upload://asdikajsdiasds.png), will be used as the public interface for access to uploads in raw.

Once that is done, we will only have to remap the urls in the uploads table and do a rebake of affected posts.

RGJ · 5월 30, 2019, 7:30오전

“only” have to do a rebake… yes, on one hand I think it’s good to get rid of the hacky remaps, on the other hand it seems like the backup/restore and migrate from/to S3 processes are getting more and more complicated and hard.

See also a number of issues in Migrate_from_s3 problems which are kind of related but they’re not all covered by your proposed solution.

sam · 5월 30, 2019, 7:52오전

I do not think so, the moves will get significantly simpler. Walk the uploads table and upload all files to new s3 bucket, rebake and you are done.

They do though become more expensive computation wise cause a rebake is required.

This is a cost I think is worth bearing given we get a correct move at the end of the process. But, we have the flexibility of still doing database replace as we do today for the cooked column and post revisions if that is how we roll, the key change in the process is that we leave posts.raw alone and then have 0% of corruption there. posts.raw and posts_uploads auto generated map will always be stable.

RGJ · 5월 30, 2019, 8:00오전

Ok - good point.

I think a significant improvement would be to walk the uploads table instead of posts (see the topic I linked above) and that would indeed leave the flexibility to replace in posts.cooked

codinghorror · 11월 21, 2019, 4:20오후

이 문제 아직 남아 있나요 @sam?

sam · 11월 21, 2019, 11:02오후

로 마크다운에서 upload:// 링크를 사용하면 이 문제를 많이 해결해 줍니다. @gerhard가 수정했는지 확실하지 않아서, 복원 후 완전한 재번역을 큐에 넣을지 고민 중입니다. (그렇게 해야 한다고 생각합니다)

gerhard · 11월 21, 2019, 11:40오후

상황에 따라 다릅니다. 최적화된 이미지가 백업에 포함되지 않는 경우(기본 설정)에는 업로드가 포함된 모든 게시물에 대해 재배킹을 수행합니다. 그렇지 않은 경우에는 리맵에만 의존합니다.

sam · 11월 21, 2019, 11:43오후

무조건적으로 다시 베이킹하도록 변경하는 것이 더 안전할 것 같습니다.

gerhard · 11월 21, 2019, 11:45오후

네, 방금 변경했습니다. 백업 및 복원 대규모 리팩토링에 포함될 예정입니다.

글		댓글	조회수
False positives on "posts are not remapped to new S3 upload URL" Bug s3	5	305	6월 22, 2025
Image uploads path in posts will not change after rebake and remap Product Support	3	1377	4월 6, 2020
Migrate_to_S3 Fails on Rebake Self-hosting	17	2830	9월 10, 2022
Bizarre Problems with migrate_from_s3 Development	2	1218	5월 2, 2018
S3 migration issues- some images got pointed to S3, but with the wrong URL format Product Support	5	1607	12월 29, 2018

Upload remapping is a bit too enthousiastic

Related topics