SubscriptionSet should not hold any database resources #5150
Conversation
|
@nirinchev, this draft PR is something I had stashed away for making SubscriptionSets not hold any database resources. MutableSubscriptionSets still need to hold on to a write transaction and an Obj, but the immutable SubscriptionSet can just be plain old data. |
src/realm/sync/subscriptions.cpp
Outdated
| , m_name(static_cast<std::string_view>(obj.get<StringData>(parent->m_sub_keys->name))) | ||
| , m_object_class_name(static_cast<std::string_view>(obj.get<StringData>(parent->m_sub_keys->object_class_name))) | ||
| , m_query_string(static_cast<std::string_view>(obj.get<StringData>(parent->m_sub_keys->query_str))) |
There was a problem hiding this comment.
Do we need the static cast to string_view here? Reading this before the hpp file changes, I thought these members were string_views, which would be wrong.
There was a problem hiding this comment.
Ah, you're right. I wasn't sure what the obj.get would actually do. It looks like it will call the std::string conversion operator to convert to a std::string which is the new type of these members.
src/realm/sync/subscriptions.cpp
Outdated
| @@ -429,7 +445,7 @@ SubscriptionSet MutableSubscriptionSet::commit() && | |||
|
|
|||
| std::string SubscriptionSet::to_ext_json() const | |||
| { | |||
| if (!m_obj.is_valid()) { | |||
| if (!m_subs.empty()) { | |||
There was a problem hiding this comment.
| if (!m_subs.empty()) { | |
| if (m_subs.empty()) { |
jbreams
force-pushed
the
jbr/inmemory_sub_sets
branch
from
33794bd
to
7ce288a
Compare
|
For posterity's sake: I ran the FLX evergreen tests with the known-to-fail client reset tests disabled here https://spruce.mongodb.com/version/61dc97640ae6060efde64d1e/tasks and they passed just fine. |
What, How & Why?
This reworks the implementation of
SubscriptionSet/MutableSubscriptionSetso thatSubscriptionSetdoes not actually hold on to any database resources and we can avoid pinning old versions of the database if a user holds on to aSubscriptionSetfor a long time.This simplified a bunch of things - like we don't need to maintain our own iterator implementation anymore, we can just re-use
std::vector's iterators. But it also made the commit method do actual serialization work in addition to just committing.This also lets us handle the
Supercededstate a little more gracefully. Before, if you calledrefresh()on aSupercededSubscriptonSetit would throw aKeyNotFounderror, and now it will properly update the state info.☑️ ToDos