-
Notifications
You must be signed in to change notification settings - Fork 165
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
RCORE-2224 clean up an unused read lock on subscription sets #7949
Conversation
test/test_sync_subscriptions.cpp
Outdated
Query query_a(read_tr->get_table("class_a")); | ||
query_a.greater_equal(fixture.bar_col, int64_t(1)); | ||
|
||
size_t num_versions_before_subscription = fixture.db->get_number_of_versions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows build failures are because get_number_of_versions is a int64_t, not size_t.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Swift SDK always destroys the MutableSubscriptionSet immediately after calling commit() so this won't make a difference there, but other SDKs may be different and seems like a reasonable change regardless.
Pull Request Test Coverage Report for Build james.stone_583Details
💛 - Coveralls |
I found this unnecessary version pin when inspecting the version locks while downloading a large bootstrap. Releasing it should improve resource utilization, especially for large sync'd Realms. I'm not sure how long SDKs hold onto the subscriptions for so this change may or may not help in practice.
Fixes #7946
☑️ ToDos
bindgen/spec.yml
, if public C++ API changed