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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Enhancements
* <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
* Improve performance of rolling back write transactions after making changes. If no KVO observers are used this is now constant time rather than taking time proportional to the number of changes to be rolled back. Rollbacks with KVO observers are 10-20% faster. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
* Enable PBS to FLX Migration feature that will migrate the local realm to use FLX if the server has been migrated to FLX. ([#6342](https://github.com/realm/realm-core/issues/6342))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this entry should be about the feature itself (not about enabling a feature) and also a proper link to the feature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, there isn't an issue specifically for the feature, since github issues aren't created for epics. I can update the comment to reflect the feature, rather than just enabling it..

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe @bmunkholm can work some magic


### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
Expand All @@ -22,6 +23,7 @@
* Simplify the non-sync replication log by emitting the same instruction type for all three types of collections rather than different instructions per collection type. This has no functional effect but eliminates some duplicated code. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
* Remove TransactionChangeInfo::track_all, which was only ever used by the global notifier. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
* Delete util::InputStream and rename util::NoCopyInputStream to util::InputStream.
* Bump the sync protocol version to v8 ([PR #6549](https://github.com/realm/realm-core/pull/6549))

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

Expand Down
23 changes: 0 additions & 23 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 @@ -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
12 changes: 0 additions & 12 deletions src/realm/sync/protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,17 @@ namespace sync {
//
constexpr int get_current_protocol_version() noexcept
{
#ifdef REALM_SYNC_PROTOCOL_V8
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
12 changes: 1 addition & 11 deletions test/object-store/sync/flx_migration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,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 = 300; // 5 minutes, for now, since it sometimes takes longer than 90 seconds
try {
timed_sleeping_wait_for(
[&] {
Expand Down Expand Up @@ -240,8 +240,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 @@ -786,13 +784,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
7 changes: 7 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,13 @@ TEST_CASE("flx: writes work without waiting for sync", "[sync][flx][app]") {
});
}

TEST_CASE("flx: verify PBS/FLX websocket protocol number and prefix", "[sync][flx]") {
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