RCORE-2203 Merge Role Changes feature branch into Core master #7897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 23 commits
June 11, 2024 13:13
…7440) * First round of changes for server-initiated bootstraps * Added test for role change bootstraps * Updated test for handle role bootstraps * Updated baas/baasaas to use branch with fixes * Updated test to verify bootstrap actually occurred * Fixed tsan warning * Updates from review; added comments to clarify bootstrap detection logic * Reworked test to fix msvc failure * Reverted baas branch to master and protocol version to 12 * Added comments to changes needed when merging to master; update baas version to not use master * Pulled over changes from other branch and tweaking download params * Refactored tests to validate different bootstrap types * Address test failures * Updated tests to get passing using the server params * Updated to support new batch_state protocol changes; updated tests * Updated role change tests and merged test from separate PR * Fixed issue with flx query verion 0 not being treated as a bootstrap * Cleaned up the tests a bit and reworked query version 0 handling * Updates from review; updated batch_state for schema bootstraps * Removed extra mutex in favor of state machine's mutex * Increased timeout when waiting for app initial sync to complete * Updated role change test to use test commands * Update resume and ident message handling * Updated future waits for the pause/resume test command * Added session connected event for when session multiplexing is disabled * Updates from review; updated baas commit to include timing fix * Added wait_until() to state machine to wait for callback; updated role change test
* Moved role change tests to separate test file * Fixed building of new flx_role_change.cpp file * Added local changes w/role bootstrap test - fixed exception in subscription store during server initiated boostrap * Updated local change test to include valid offline writes during role change * Added role change test during initial schema bootstrap * Wrapped up role change during bootstrap tests * Removed debug statments to fix thread sanitizer * Updated sub state comments and reverted a minor change * Refactored role change tests and broke out into 2 separate test cases * Moved harness from a global to a static var in each test case * Reverted resetting the bootstrapping subscription state back to Pending * Updated baas to use protocol v14 and removed the feature flag for role change bootstraps * Removed left over code in statement... * Updated baasaas version to be a cached version * Updated baasaas githash and reordered role change during bootstrap to check for role change bootstrap as first validation step * Minor updates to reuse the verify_records() fcn * RCORE-2174 Bootstrap store is not being reset if initial subscription bootstrap is interrupted by role change (#7831) * Updated pending bootstrap store to be processed (applied or cleared) when the session or connection is restarted without restarting the Sync Session
…#7840) * re-applied changes after base branch was merged to feature branch * Updates to address test failures * Disable role change check during fresh realm download * Updated comments for clarity * Update from review * Update from review * Updates from review; added a bunch of comments to test * Updates to role change tests per review comments * removed ostream support for SyncClientHookEvent
Pull Request Test Coverage Report for Build michael.wilkersonbarker_1285Details
💛 - Coveralls |
ironage
approved these changes
Jul 18, 2024
tgoyne
reviewed
Jul 18, 2024
| @@ -1233,6 +1200,10 @@ void SessionWrapper::on_flx_sync_progress(int64_t new_version, DownloadBatchStat | |||
| if (!has_flx_subscription_store()) { | |||
| return; | |||
| } | |||
| // Is this a server-initiated bootstrap? Skip notifying the subscription store | |||
| if (new_version == m_flx_active_version) { | |||
There was a problem hiding this comment.
This is also checked again in the DownloadBatchState::LastInBatch case, so one of these is redundant.
It appears that if the server sends us a bootstrap for a version other than what we think the active query version is we'll hit an assertion failure rather than reporting a protocol invariant failure.
src/realm/sync/config.hpp
Outdated
| @@ -123,6 +123,7 @@ enum class ClientResyncMode : unsigned char { | |||
| RecoverOrDiscard, | |||
| }; | |||
|
|
|||
| // Also update sync_test_utils.hpp when adding new values | |||
There was a problem hiding this comment.
It might be worth doing something similar to what instructions.hpp does with REALM_FOR_EACH_INSTRUCTION_TYPE to avoid having to update multiple places?
| @@ -5,3 +5,4 @@ ZLIB_VERSION: 1.2.13 | |||
| # https://github.com/10gen/baas/commits | |||
| # 2f308db is 2024 July 10 | |||
| BAAS_VERSION: 2f308db6f65333728a101d1fecbb792f9659a5ce | |||
There was a problem hiding this comment.
is the server using v14 already? and more specifically, do we need to bump this version?
There was a problem hiding this comment.
Yes, the server has already been released - this is the same version that we are using on master (updated last week) which includes these changes.
danieltabacaru
approved these changes
Jul 18, 2024
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What, How & Why?
Merging the "Handle Role Changes without Client Reset" feature to Core Master from the feature branch.
Fixes #7896
☑️ ToDos
[ ] C-API, if public C++ API changed[ ]bindgen/spec.yml, if public C++ API changed