diff --git a/CHANGELOG.md b/CHANGELOG.md index 933f9009d0..1888de308a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* None. +* The events library would attempt to upload backup files created as part of file format upgrades, causing backup copies of those backups to be made, looping until the maximum file name size was reached (since v11.17.0). ### Breaking changes * None. diff --git a/src/realm/backup_restore.cpp b/src/realm/backup_restore.cpp index 8070642af0..9b6d9e33f6 100644 --- a/src/realm/backup_restore.cpp +++ b/src/realm/backup_restore.cpp @@ -80,6 +80,11 @@ BackupHandler::BackupHandler(const std::string& path, const VersionList& accepte m_delete_versions = to_be_deleted; } +std::string BackupHandler::get_prefix() const +{ + return m_prefix; +} + bool BackupHandler::must_restore_from_backup(int current_file_format_version) const { if (current_file_format_version == 0) diff --git a/src/realm/backup_restore.hpp b/src/realm/backup_restore.hpp index e0a258546a..3222b1c128 100644 --- a/src/realm/backup_restore.hpp +++ b/src/realm/backup_restore.hpp @@ -34,7 +34,7 @@ class BackupHandler { void restore_from_backup(); void cleanup_backups(); void backup_realm_if_needed(int current_file_format_version, int target_file_format_version); - std::string get_prefix(); + std::string get_prefix() const; static std::string get_prefix_from_path(const std::string& path); // default lists of accepted versions and backups to delete when they get old enough diff --git a/src/realm/object-store/audit.mm b/src/realm/object-store/audit.mm index ceae5c3894..85313cf96c 100644 --- a/src/realm/object-store/audit.mm +++ b/src/realm/object-store/audit.mm @@ -655,6 +655,7 @@ explicit AuditRealmPool(Private, std::shared_ptr user, const AuditConf void open_new_realm() REQUIRES(!m_mutex); void wait_for_upload(std::shared_ptr) REQUIRES(m_mutex); void scan_for_realms_to_upload() REQUIRES(!m_mutex); + bool clean_up_backup(const std::string& file_name); std::string prefixed_partition(std::string const& partition); }; @@ -814,6 +815,8 @@ explicit AuditRealmPool(Private, std::shared_ptr user, const AuditConf while (dir.next(file_name)) { if (!StringData(file_name).ends_with(".realm")) continue; + if (clean_up_backup(file_name)) + continue; std::string path = m_path_root + file_name; if (m_open_paths.count(path)) { @@ -846,6 +849,37 @@ explicit AuditRealmPool(Private, std::shared_ptr user, const AuditConf m_cv.notify_all(); } +bool AuditRealmPool::clean_up_backup(const std::string& file_name) +{ + auto pos = file_name.find(".backup."); + if (pos == std::string::npos) + return false; + + auto base_name = StringData(file_name.c_str(), file_name.find('.')); + auto base_realm_path = util::format("%1%2.realm", m_path_root, base_name); + + // If the file which this was a backup of no longer exists we no longer need + // the backup, and the backup won't be cleaned up by the normal code path as + // that happens when the base file is opened. + if (!util::File::exists(base_realm_path)) { + auto path = m_path_root + file_name; + m_logger->info("Events: Removing stale backup of uploaded file at '%1'", path); + util::File::remove(path); + return true; + } + + pos = file_name.find(".backup.", pos + 1); + if (pos == std::string::npos) + return true; + + // .backup appears multiple times, so this was a backup of a backup and + // should be removed even though the base Realm still exists + auto path = m_path_root + file_name; + m_logger->info("Events: Removing backup of a backup at '%1'", path); + util::File::remove(path); + return true; +} + void AuditRealmPool::open_new_realm() { ObjectSchema schema = {"AuditEvent", diff --git a/test/object-store/audit.cpp b/test/object-store/audit.cpp index c4fad02fb3..3e3432f015 100644 --- a/test/object-store/audit.cpp +++ b/test/object-store/audit.cpp @@ -1542,7 +1542,7 @@ TEST_CASE("audit realm sharding", "[sync][pbs][audit]") { std::string file_name; util::DirScanner dir(root); size_t file_count = 0; - size_t unlocked_file_count = 0; + std::vector unlocked_files; while (dir.next(file_name)) { if (!StringData(file_name).ends_with(".realm")) continue; @@ -1551,7 +1551,7 @@ TEST_CASE("audit realm sharding", "[sync][pbs][audit]") { // than it. 1 MB errs on the side of never getting spurious failures. REQUIRE(util::File::get_size_static(root + "/" + file_name) < 1024 * 1024); if (DB::call_with_lock(root + "/" + file_name, [](auto&) {})) { - ++unlocked_file_count; + unlocked_files.push_back(file_name); } } // The exact number of shards is fuzzy due to the combination of the @@ -1561,7 +1561,16 @@ TEST_CASE("audit realm sharding", "[sync][pbs][audit]") { // There should be exactly two files open still: the one we're currently // writing to, and the first one which we wrote and are waiting for the // upload to complete. - REQUIRE(unlocked_file_count == file_count - 2); + REQUIRE(unlocked_files.size() == file_count - 2); + + // Create a backup copy of each of the unlocked files which should be cleaned up + for (auto& file : unlocked_files) { + BackupHandler handler(root + "/" + file, {}, {}); + handler.backup_realm_if_needed(23, 24); + // Set the version field in the backup file to 23 so that opening it + // won't accidentally work + util::File(handler.get_prefix() + "v23.backup.realm", util::File::mode_Update).write(12, "\x17"); + } auto get_sorted_events = [&] { auto events = get_audit_events(test_session, false);