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

Bump the protocol version to v8 #6549

Merged
merged 9 commits into from
Apr 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

### Enhancements
* <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
* None.
* PBS to FLX Migration for migrating a client app that uses partition based sync to use flexible sync under the hood if the server has been migrated to flexible sync. ([#6554](https://github.com/realm/realm-core/issues/6554))

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
Expand All @@ -17,7 +17,7 @@
-----------

### Internals
* None.
* Bump the sync protocol version to v8 ([PR #6549](https://github.com/realm/realm-core/pull/6549))

----------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion dependencies.list
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ PACKAGE_NAME=realm-core
VERSION=13.9.4
OPENSSL_VERSION=3.0.8
WIN32_ZLIB_VERSION=1.2.13
MDBREALM_TEST_SERVER_TAG=2023-04-13
MDBREALM_TEST_SERVER_TAG=2023-04-29
25 changes: 1 addition & 24 deletions evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ functions:
set_cmake_var realm_vars REALM_TEST_LOGGING BOOL On
set_cmake_var realm_vars REALM_TEST_LOGGING_LEVEL STRING "debug"

if [ -n "${enable_sync_v8|}" ]; then
set_cmake_var realm_vars REALM_SYNC_PROTOCOL_V8 BOOL On
fi

GENERATOR="${cmake_generator}"
if [ -z "${cmake_generator|}" ]; then
GENERATOR="Ninja Multi-Config"
Expand Down Expand Up @@ -634,7 +630,7 @@ tasks:
export DEVELOPER_DIR="${xcode_developer_dir}"
fi

./evergreen/install_baas.sh -w ./baas-work-dir -b cfda5c63e6910091853d3fb1085dcf45396279bb 2>&1 | tee install_baas_output.log
./evergreen/install_baas.sh -w ./baas-work-dir -b 4da4c3b3e9083ea834522bdf67a2e129fd701d86 2>&1 | tee install_baas_output.log
fi

- command: shell.exec
Expand Down Expand Up @@ -923,25 +919,6 @@ buildvariants:
- ubuntu2004-large
- name: long-running-tests

- name: ubuntu2004-asan-sync-v8
display_name: "Ubuntu 20.04 x86_64 SyncV8 (Clang 11 ASAN)"
run_on: ubuntu2004-small
expansions:
clang_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/clang%2Bllvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04.tar.xz"
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-linux-x86_64.tar.gz"
cmake_bindir: "./cmake_binaries/bin"
fetch_missing_dependencies: On
run_tests_against_baas: On
enable_asan: On
enable_sync_v8: On
c_compiler: "./clang_binaries/bin/clang"
cxx_compiler: "./clang_binaries/bin/clang++"
llvm_symbolizer: "./clang_binaries/bin/llvm-symbolizer"
tasks:
- name: object-store-tests-v8
distros:
- ubuntu2004-large

- name: ubuntu2004-tsan
display_name: "Ubuntu 20.04 x86_64 (Clang 11 TSAN)"
run_on: ubuntu2004-small
Expand Down
3 changes: 2 additions & 1 deletion src/realm/sync/noinst/protocol_codec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ void ClientProtocol::make_flx_bind_message(int protocol_version, OutputBuffer& o
const nlohmann::json& json_data, const std::string& signed_user_token,
bool need_client_file_ident, bool is_subserver)
{
static_cast<void>(protocol_version);
std::string json_data_stg;
// Protocol version v8 and above accepts stringified json_data for the first data argument
if (protocol_version >= 8 && !json_data.empty()) {
if (!json_data.empty()) {
json_data_stg = json_data.dump();
}

Expand Down
14 changes: 2 additions & 12 deletions src/realm/sync/protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,19 @@ namespace sync {
//
constexpr int get_current_protocol_version() noexcept
{
#ifdef REALM_SYNC_PROTOCOL_V8
// Also update the current protocol version test in flx_sync.cpp when
// updating this value
return 8;
#else
return 7;
#endif // REALM_SYNC_PROTOCOL_V8
}

constexpr std::string_view get_pbs_websocket_protocol_prefix() noexcept
{
#ifdef REALM_SYNC_PROTOCOL_V8
return "com.mongodb.realm-sync#";
#else
return "com.mongodb.realm-sync/";
#endif // REALM_SYNC_PROTOCOL_V8
}

constexpr std::string_view get_flx_websocket_protocol_prefix() noexcept
{
#ifdef REALM_SYNC_PROTOCOL_V8
return "com.mongodb.realm-query-sync#";
#else
return "com.mongodb.realm-query-sync/";
#endif // REALM_SYNC_PROTOCOL_V8
}

enum class SyncServerMode { PBS, FLX };
Expand Down
4 changes: 0 additions & 4 deletions test/object-store/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ add_bundled_test(ObjectStoreTests)
if(REALM_ENABLE_SYNC)
target_link_libraries(ObjectStoreTests SyncServer)
option(REALM_ENABLE_AUTH_TESTS "" OFF)
option(REALM_SYNC_PROTOCOL_V8 "" OFF)
if(REALM_ENABLE_AUTH_TESTS)
if(NOT REALM_MONGODB_ENDPOINT)
message(FATAL_ERROR "REALM_MONGODB_ENDPOINT must be set when specifying REALM_ENABLE_AUTH_TESTS.")
Expand All @@ -108,9 +107,6 @@ if(REALM_ENABLE_SYNC)
find_package(CURL REQUIRED)
target_link_libraries(ObjectStoreTests CURL::libcurl)
endif()
if(REALM_SYNC_PROTOCOL_V8)
target_compile_definitions(ObjectStoreTests PRIVATE REALM_SYNC_PROTOCOL_V8=1)
endif()
endif()

if(REALM_TEST_LOGGING)
Expand Down
89 changes: 36 additions & 53 deletions test/object-store/sync/flx_migration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include <catch2/catch_all.hpp>
#include <chrono>


#if REALM_ENABLE_SYNC
#if REALM_ENABLE_AUTH_TESTS

Expand All @@ -40,7 +39,7 @@ static void trigger_server_migration(const AppSession& app_session, MigrationMod
else
return "FLX->PBS Server rollback";
}();
const int duration = 300; // 5 minutes, for now, since it sometimes takes longet than 90 seconds
const int duration = 480; // 8 minutes, for now, since it sometimes takes longer than 300 seconds
try {
timed_sleeping_wait_for(
[&] {
Expand Down Expand Up @@ -240,8 +239,6 @@ TEST_CASE("Test server migration and rollback", "[flx][migration]") {
}
}

#ifdef REALM_SYNC_PROTOCOL_V8

TEST_CASE("Test client migration and rollback", "[flx][migration]") {
std::shared_ptr<util::Logger> logger_ptr =
std::make_shared<util::StderrLogger>(realm::util::Logger::Level::TEST_LOGGING_LEVEL);
Expand Down Expand Up @@ -319,32 +316,32 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
// Primary key of the object to recover
auto obj_id = ObjectId::gen();

// Keep this realm around for after the revert to PBS
auto outer_realm = Realm::get_shared_realm(config);
REQUIRE(!wait_for_upload(*outer_realm));
REQUIRE(!wait_for_download(*outer_realm));

// Wait to upload the data
{
auto realm = Realm::get_shared_realm(config);

REQUIRE(!wait_for_upload(*realm));
REQUIRE(!wait_for_download(*realm));

auto table = realm->read_group().get_table("class_Object");
auto table = outer_realm->read_group().get_table("class_Object");
REQUIRE(table->size() == 5);

// Close the sync session and make a change. This will be recovered by the migration.
realm->sync_session()->force_close();
realm->begin_transaction();
realm->read_group()
// Pause the sync session and make a change.
// This will be recovered when it is resumed after the migration.
outer_realm->sync_session()->pause();
outer_realm->begin_transaction();
outer_realm->read_group()
.get_table("class_Object")
->create_object_with_primary_key(obj_id)
.set("string_field", "partition-set-during-sync-upload");
realm->commit_transaction();
outer_realm->commit_transaction();
}

// Migrate to FLX
trigger_server_migration(session.app_session(), MigrateToFLX, logger_ptr);

// Keep this realm around for after the revert to PBS
auto outer_realm = Realm::get_shared_realm(config);

// Resume the session and verify the additional object was uploaded after the migration
outer_realm->sync_session()->resume();
REQUIRE(!wait_for_upload(*outer_realm));
REQUIRE(!wait_for_download(*outer_realm));

Expand All @@ -363,11 +360,11 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
auto object_table = outer_realm->read_group().get_table("class_Object");
auto pending_object = object_table->get_object_with_primary_key(obj_id);
REQUIRE(pending_object.get<String>("string_field") == "partition-set-during-sync-upload");

// Close the session and create a dummy subscription with a notification to verify it has been cancelled
outer_realm->sync_session()->pause();
}

// Pause the sync session so a pending subscription and object can be created
// before processing the rollback
outer_realm->sync_session()->pause();
util::Future<sync::SubscriptionSet::State> new_subs_future = [&] {
auto sub_store = outer_realm->sync_session()->get_flx_subscription_store();
auto mut_subs = sub_store->get_active().make_mutable_copy();
Expand All @@ -379,13 +376,6 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
return new_subs.get_state_change_notification(sync::SubscriptionSet::State::Complete);
}();

// Wait for the object to be written to Atlas/MongoDB before rollback, otherwise it may be lost
reset_utils::wait_for_object_to_persist_to_atlas(session.app()->current_user(), session.app_session(), "Object",
{{"_id", obj_id}});

// Roll back to PBS
trigger_server_migration(session.app_session(), RollbackToPBS, logger_ptr);

// Add a local object while the session is paused. This will be recovered when connecting after the rollback.
{
outer_realm->begin_transaction();
Expand All @@ -396,6 +386,13 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
outer_realm->commit_transaction();
}

// Wait for the object to be written to Atlas/MongoDB before rollback, otherwise it may be lost
reset_utils::wait_for_object_to_persist_to_atlas(session.app()->current_user(), session.app_session(), "Object",
{{"_id", obj_id}});

// Roll back to PBS
trigger_server_migration(session.app_session(), RollbackToPBS, logger_ptr);

// Connect after rolling back to PBS
outer_realm->sync_session()->resume();
REQUIRE(!wait_for_upload(*outer_realm));
Expand All @@ -419,21 +416,17 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
REALM_ASSERT(result.get_value() == sync::SubscriptionSet::State::Superseded);
}

outer_realm.reset();

// Migrate back to FLX
// Migrate back to FLX - and keep the realm session open
trigger_server_migration(session.app_session(), MigrateToFLX, logger_ptr);

REQUIRE(!wait_for_upload(*outer_realm));
REQUIRE(!wait_for_download(*outer_realm));

// Verify data has been sync'ed and there is only 1 subscription for the Object table
{
auto realm = Realm::get_shared_realm(config);

REQUIRE(!wait_for_upload(*realm));
REQUIRE(!wait_for_download(*realm));

auto table = realm->read_group().get_table("class_Object");
auto table = outer_realm->read_group().get_table("class_Object");
REQUIRE(table->size() == 7);
auto sync_session = realm->sync_session();
auto sync_session = outer_realm->sync_session();
REQUIRE(sync_session);
auto sub_store = sync_session->get_flx_subscription_store();
REQUIRE(sub_store);
Expand All @@ -442,16 +435,14 @@ TEST_CASE("Test client migration and rollback with recovery", "[flx][migration]"
REQUIRE(active_subs.find("flx_migrated_Object"));
}

// Roll back to PBS once again
// Roll back to PBS once again - and keep the realm session open
trigger_server_migration(session.app_session(), RollbackToPBS, logger_ptr);

{
auto realm = Realm::get_shared_realm(config);

REQUIRE(!wait_for_upload(*realm));
REQUIRE(!wait_for_download(*realm));
REQUIRE(!wait_for_upload(*outer_realm));
REQUIRE(!wait_for_download(*outer_realm));

auto table = realm->read_group().get_table("class_Object");
{
auto table = outer_realm->read_group().get_table("class_Object");
REQUIRE(table->size() == 7);
}
}
Expand Down Expand Up @@ -786,13 +777,5 @@ TEST_CASE("New table is synced after migration", "[flx][migration]") {
}
}

TEST_CASE("Validate protocol v8 features", "[flx][migration]") {
REQUIRE(sync::get_current_protocol_version() >= 8);
REQUIRE("com.mongodb.realm-sync#" == sync::get_pbs_websocket_protocol_prefix());
REQUIRE("com.mongodb.realm-query-sync#" == sync::get_flx_websocket_protocol_prefix());
}

#endif // REALM_SYNC_PROTOCOL_V8

#endif // REALM_ENABLE_AUTH_TESTS
#endif // REALM_ENABLE_SYNC
9 changes: 9 additions & 0 deletions test/object-store/sync/flx_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1588,6 +1588,15 @@ TEST_CASE("flx: writes work without waiting for sync", "[sync][flx][app]") {
});
}

TEST_CASE("flx: verify PBS/FLX websocket protocol number and prefixes", "[sync][flx]") {
// Update the expected value whenever the protocol version is updated - this ensures
// that the current protocol version does not change unexpectedly.
REQUIRE(8 == sync::get_current_protocol_version());
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will break when we bump the version again. You should keep the initial assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a check to verify that the current protocol version is not updated unexpectedly - I added comments around these to make sure they are updated when the protocol version is bumped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, my point is that going forward the checks should still be valid unless we make other changes to the prefix and we should then assert on the new version.

// This was updated in Protocol V8 to use '#' instead of '/' to support the Web SDK
REQUIRE("com.mongodb.realm-sync#" == sync::get_pbs_websocket_protocol_prefix());
REQUIRE("com.mongodb.realm-query-sync#" == sync::get_flx_websocket_protocol_prefix());
}

TEST_CASE("flx: subscriptions persist after closing/reopening", "[sync][flx][app]") {
FLXSyncTestHarness harness("flx_bad_query");
SyncTestFile config(harness.app()->current_user(), harness.schema(), SyncConfig::FLXSyncEnabled{});
Expand Down
5 changes: 0 additions & 5 deletions test/test_sync_protocol_codec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@ TEST(Protocol_Codec_Bind_FLX)
json_data["valA"] = 123;
json_data["valB"] = "something";

out.reset();
expected_out_string = "bind 234888 0 6 0 1\ntoken1";
protocol.make_flx_bind_message(7, out, 234888, json_data, "token1", false, true);
compare_out_string(expected_out_string, out, test_context);

out.reset();
expected_out_string = "bind 345888 0 6 1 0\ntoken2";
protocol.make_flx_bind_message(8, out, 345888, {}, "token2", true, false);
Expand Down