From 930be301439ef1e6c28c100eef027cb67dfb0c62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 27 Jan 2023 09:25:41 +0100 Subject: [PATCH] Fix handling of work limit in online compaction The work limit could wrap around resulting in an unlimited run, which could take a long time to finish. --- CHANGELOG.md | 1 + src/realm/db.cpp | 4 ++-- src/realm/group_writer.cpp | 6 ++---- src/realm/group_writer.hpp | 12 ++++++++++++ src/realm/transaction.cpp | 21 +++++++++------------ src/realm/transaction.hpp | 2 +- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf2645b830f..db175b2b9fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * Properties and types not present in the requested schema would be missing from the reported schema in several scenarios, such as if the Realm was being opened with a different schema version than the persisted one, and if the new tables or columns were added while the Realm instance did not have an active read transaction ([PR #6211](https://github.com/realm/realm-core/pull/6211), since v13.2.0). * If a client reset w/recovery or discard local is interrupted while the "fresh" realm is being downloaded, the sync client may crash with a MultpleSyncAgents exception ([#6217](https://github.com/realm/realm-core/issues/6217), since v11.13.0) * Changesets from the server sent during FLX bootstrapping that are larger than 16MB can cause the sync client to crash with a LogicError (PR [#6218](https://github.com/realm/realm-core/pull/6218), since v12.0.0) +* Online compaction may cause a single commit to take a long time ([#6245](https://github.com/realm/realm-core/pull/6245), since v13.0.0) ### Breaking changes * `SyncSession::log_out()` has been renamed to `SyncSession::force_close()` to reflect what it actually does ([#6183](https://github.com/realm/realm-core/pull/6183)) diff --git a/src/realm/db.cpp b/src/realm/db.cpp index 59928e1de28..d30cdbe5b4d 100644 --- a/src/realm/db.cpp +++ b/src/realm/db.cpp @@ -2225,8 +2225,8 @@ void DB::low_level_commit(uint_fast64_t new_version, Transaction& transaction, b if (auto limit = out.get_evacuation_limit()) { // Get a work limit based on the size of the transaction we're about to commit - // Assume at least 4K on top of that for the top arrays - size_t work_limit = 4 * 1024 + m_alloc.get_commit_size() / 2; + // Add 4k to ensure progress on small commits + size_t work_limit = m_alloc.get_commit_size() / 2 + out.get_free_list_size() + 0x1000; transaction.cow_outliers(out.get_evacuation_progress(), limit, work_limit); } diff --git a/src/realm/group_writer.cpp b/src/realm/group_writer.cpp index 33c85f46042..c6082464b87 100644 --- a/src/realm/group_writer.cpp +++ b/src/realm/group_writer.cpp @@ -679,10 +679,8 @@ ref_type GroupWriter::write_group() // bigger databases the space required for free lists will be relatively less. max_free_list_size += 10; - // If current size is less than 128 MB, the database need not expand above 2 GB - // which means that the positions and sizes can still be in 32 bit. - int size_per_entry = (m_logical_size < 0x8000000 ? 8 : 16) + 8; - size_t max_free_space_needed = Array::get_max_byte_size(top.size()) + size_per_entry * max_free_list_size; + size_t max_free_space_needed = + Array::get_max_byte_size(top.size()) + size_per_free_list_entry() * max_free_list_size; ALLOC_DBG_COUT(" Allocating file space for freelists:" << std::endl); // Reserve space for remaining arrays. We ask for some extra bytes beyond the diff --git a/src/realm/group_writer.hpp b/src/realm/group_writer.hpp index ec63a31ce6e..41e8b9b4da8 100644 --- a/src/realm/group_writer.hpp +++ b/src/realm/group_writer.hpp @@ -116,6 +116,11 @@ class GroupWriter : public _impl::ArrayWriterBase { return m_backoff ? 0 : m_evacuation_limit; } + size_t get_free_list_size() + { + return m_free_positions.size() * size_per_free_list_entry(); + } + std::vector& get_evacuation_progress() { return m_evacuation_progress; @@ -254,6 +259,13 @@ class GroupWriter : public _impl::ArrayWriterBase { /// Debug helper - extends the TopRefMap with list of reachable blocks void map_reachable(); + + size_t size_per_free_list_entry() const + { + // If current size is less than 128 MB, the database need not expand above 2 GB + // which means that the positions and sizes can still be in 32 bit. + return (m_logical_size < 0x8000000 ? 8 : 16) + 8; + } }; diff --git a/src/realm/transaction.cpp b/src/realm/transaction.cpp index 346560f9304..814f07f8eba 100644 --- a/src/realm/transaction.cpp +++ b/src/realm/transaction.cpp @@ -933,9 +933,9 @@ void Transaction::set_transact_stage(DB::TransactStage stage) noexcept class NodeTree { public: - NodeTree(size_t evac_limit, size_t& work_limit) + NodeTree(size_t evac_limit, size_t work_limit) : m_evac_limit(evac_limit) - , m_work_limit(work_limit) + , m_work_limit(int64_t(work_limit)) , m_moved(0) { } @@ -955,18 +955,15 @@ class NodeTree { /// to point to the node we have just processed bool trv(Array& current_node, unsigned level, std::vector& progress) { + if (m_work_limit < 0) { + return false; + } if (current_node.is_read_only()) { - auto byte_size = current_node.get_byte_size(); + size_t byte_size = current_node.get_byte_size(); if ((current_node.get_ref() + byte_size) > m_evac_limit) { current_node.copy_on_write(); m_moved++; - if (m_work_limit > byte_size) { - m_work_limit -= byte_size; - } - else { - m_work_limit = 0; - return false; - } + m_work_limit -= byte_size; } } @@ -998,12 +995,12 @@ class NodeTree { private: size_t m_evac_limit; - size_t m_work_limit; + int64_t m_work_limit; size_t m_moved; }; -void Transaction::cow_outliers(std::vector& progress, size_t evac_limit, size_t& work_limit) +void Transaction::cow_outliers(std::vector& progress, size_t evac_limit, size_t work_limit) { NodeTree node_tree(evac_limit, work_limit); if (progress.empty()) { diff --git a/src/realm/transaction.hpp b/src/realm/transaction.hpp index ce3e98942ca..119babc2a8e 100644 --- a/src/realm/transaction.hpp +++ b/src/realm/transaction.hpp @@ -188,7 +188,7 @@ class Transaction : public Group { void complete_async_commit(); void acquire_write_lock() REQUIRES(!m_async_mutex); - void cow_outliers(std::vector& progress, size_t evac_limit, size_t& work_limit); + void cow_outliers(std::vector& progress, size_t evac_limit, size_t work_limit); void close_read_with_lock() REQUIRES(!m_async_mutex, db->m_mutex); DBRef db;