Skip to content

Commit

Permalink
Skip backup files when scanning for audit Realms to upload
Browse files Browse the repository at this point in the history
Backup realms from file format upgrades are placed next to the original file,
so the audit realm pool will find the backup and attempt to upload it,
triggering another upgrade and backup until the filename becomes too long and
it crashes instead. It needs to instead skip the backup files and delete any
lingering backups of Realms which have already been uploaded.
  • Loading branch information
tgoyne committed Oct 8, 2024
1 parent c729fc8 commit 737b110
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 5 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](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.
Expand Down
5 changes: 5 additions & 0 deletions src/realm/backup_restore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/realm/backup_restore.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions src/realm/object-store/audit.mm
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ explicit AuditRealmPool(Private, std::shared_ptr<SyncUser> user, const AuditConf
void open_new_realm() REQUIRES(!m_mutex);
void wait_for_upload(std::shared_ptr<SyncSession>) 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);
};

Expand Down Expand Up @@ -814,6 +815,8 @@ explicit AuditRealmPool(Private, std::shared_ptr<SyncUser> 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)) {
Expand Down Expand Up @@ -846,6 +849,37 @@ explicit AuditRealmPool(Private, std::shared_ptr<SyncUser> 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",
Expand Down
15 changes: 12 additions & 3 deletions test/object-store/audit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> unlocked_files;
while (dir.next(file_name)) {
if (!StringData(file_name).ends_with(".realm"))
continue;
Expand All @@ -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
Expand All @@ -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);
Expand Down

0 comments on commit 737b110

Please sign in to comment.