Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse transaction when refreshing subscriptions after commit #8068

Merged
merged 4 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/realm/sync/subscriptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,13 +576,7 @@ SubscriptionSet MutableSubscriptionSet::commit()
m_tr->commit_and_continue_as_read();

mgr->report_progress(m_tr);

DB::VersionID commit_version = m_tr->get_version_of_current_transaction();
// release the read lock so that this instance doesn't keep a version pinned
// for the remainder of its lifetime
m_tr.reset();

return mgr->get_refreshed(m_obj.get_key(), flx_version, commit_version);
return mgr->get_refreshed(m_obj.get_key(), flx_version, std::move(m_tr));
}

std::string SubscriptionSet::to_ext_json() const
Expand Down Expand Up @@ -970,9 +964,9 @@ SubscriptionSet SubscriptionStore::get_by_version(int64_t version_id)
throw KeyNotFound(util::format("Subscription set with version %1 not found", version_id));
}

SubscriptionSet SubscriptionStore::get_refreshed(ObjKey key, int64_t version, std::optional<DB::VersionID> db_version)
SubscriptionSet SubscriptionStore::get_refreshed(ObjKey key, int64_t version, std::optional<TransactionRef> maybe_tr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you don't need std::optional here. TransactionRef is already a pointer type that can be checked for null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its clearer with the optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just that we have a tradition for checking the pointer directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, done. can you take another look?

{
auto tr = m_db->start_frozen(db_version.value_or(VersionID{}));
auto tr = maybe_tr.value_or(m_db->start_frozen());
auto sub_sets = tr->get_table(m_sub_set_table);
if (auto obj = sub_sets->try_get_object(key)) {
return SubscriptionSet(weak_from_this(), *tr, obj);
Expand Down
2 changes: 1 addition & 1 deletion src/realm/sync/subscriptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ class SubscriptionStore : public std::enable_shared_from_this<SubscriptionStore>
};

Obj get_active(const Transaction& tr);
SubscriptionSet get_refreshed(ObjKey, int64_t flx_version, std::optional<DB::VersionID> version = util::none);
SubscriptionSet get_refreshed(ObjKey, int64_t flx_version, std::optional<TransactionRef> tr = util::none);
MutableSubscriptionSet make_mutable_copy(const SubscriptionSet& set);

// Ensure the subscriptions table is properly initialized. No-op if already initialized.
Expand Down
Loading