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

Workaround issue when initiating sync on a copied realm file #4878

Merged
merged 6 commits into from
Sep 15, 2021
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 src/realm/exec/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ target_link_libraries(RealmTrawler Storage)

add_executable(RealmBrowser EXCLUDE_FROM_ALL realm_browser.cpp )
set_target_properties(RealmBrowser PROPERTIES
OUTPUT_NAME "realm-browser-6"
OUTPUT_NAME "realm-browser-10"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just remove these suffixes? What value are they adding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are adding the value that It is build into the name which version it can handle. I have to have multiple versions around so that I can open whatever realm file I get from a customer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, if it's part of your workflow then okay. I wish this were configurable. We use these tools outside of your debugging processes - e.g. the cloud team depends on them. @tkaye407 , will you need to update anything if we rename realm2json like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We currently just pin to a specific version (though we probably should be able to pick up the latest merge), but once this goes in I can update the version / name of the executable that we link to in our evergreen config. Thanks for pointing out

DEBUG_POSTFIX ${CMAKE_DEBUG_POSTFIX}
)
target_link_libraries(RealmBrowser Storage)

add_executable(Realm2JSON realm2json.cpp )
set_target_properties(Realm2JSON PROPERTIES
OUTPUT_NAME "realm2json-6"
OUTPUT_NAME "realm2json-10"
DEBUG_POSTFIX ${CMAKE_DEBUG_POSTFIX}
)
target_link_libraries(Realm2JSON Storage)
Expand Down
12 changes: 8 additions & 4 deletions src/realm/sync/noinst/client_history_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,10 @@ void ClientHistoryImpl::update_sync_progress(const SyncProgress& progress,
version_type(root.get_as_ref_or_tagged(s_progress_download_client_version_iip).get_as_int()));
REALM_ASSERT(progress.upload.client_version >=
version_type(root.get_as_ref_or_tagged(s_progress_upload_client_version_iip).get_as_int()));
REALM_ASSERT(progress.upload.last_integrated_server_version >=
version_type(root.get_as_ref_or_tagged(s_progress_upload_server_version_iip).get_as_int()));
if (progress.upload.last_integrated_server_version > 0) {
REALM_ASSERT(progress.upload.last_integrated_server_version >=
version_type(root.get_as_ref_or_tagged(s_progress_upload_server_version_iip).get_as_int()));
}

auto uploaded_bytes = std::uint_fast64_t(root.get_as_ref_or_tagged(s_progress_uploaded_bytes_iip).get_as_int());
auto previous_upload_client_version =
Expand All @@ -829,8 +831,10 @@ void ClientHistoryImpl::update_sync_progress(const SyncProgress& progress,
RefOrTagged::make_tagged(progress.latest_server_version.salt)); // Throws
root.set(s_progress_upload_client_version_iip,
RefOrTagged::make_tagged(progress.upload.client_version)); // Throws
root.set(s_progress_upload_server_version_iip,
RefOrTagged::make_tagged(progress.upload.last_integrated_server_version)); // Throws
if (progress.upload.last_integrated_server_version > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the current version of s_progres_upload_server_version_iip is zero, would this be a noop without this conditional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure I understand the question. If current value is zero and progress value is zero then it is obviously a noop. The test just makes sure that we will not overwrite a non-zero value with a zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are answering my question without understanding it 😄 . We were changing this from unconditionally setting a value to conditionally setting a value, and I wanted to verify the preconditions. Can you add a comment about why we're conditionally setting this here?

root.set(s_progress_upload_server_version_iip,
RefOrTagged::make_tagged(progress.upload.last_integrated_server_version)); // Throws
}
if (downloadable_bytes) {
root.set(s_progress_downloadable_bytes_iip,
RefOrTagged::make_tagged(*downloadable_bytes)); // Throws
Expand Down
16 changes: 13 additions & 3 deletions src/realm/sync/noinst/server_history.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,16 @@ auto ServerHistory::do_bootstrap_client_session(SaltedFileIdent client_file_iden
if (download_progress.server_version > current_server_version)
return BootstrapError::bad_download_server_version;
auto last_integrated_client_version = version_type(m_acc->cf_client_versions.get(client_file_index));
if (download_progress.last_integrated_client_version > last_integrated_client_version)
return BootstrapError::bad_download_client_version;
if (download_progress.last_integrated_client_version > last_integrated_client_version) {
if (last_integrated_client_version == 0) {
// We assume we are booting on a pre-downloaded client file
last_integrated_client_version = download_progress.last_integrated_client_version;
recip_hist_base_version = download_progress.server_version;
}
else {
return BootstrapError::bad_download_client_version;
}
}

// Validate `server_version`
{
Expand Down Expand Up @@ -1014,7 +1022,9 @@ bool ServerHistory::fetch_download_info(file_ident_type client_file_ident, Downl
download_progress = download_progress_2;
cumulative_byte_size_current = std::uint_fast64_t(cumulative_byte_size_current_2);
cumulative_byte_size_total = std::uint_fast64_t(cumulative_byte_size_total_2);
upload_progress = UploadCursor{upload_client_version, upload_server_version};
if (upload_client_version > upload_progress.client_version) {
upload_progress = UploadCursor{upload_client_version, upload_server_version};
}

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/realm/sync/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3637,7 +3637,7 @@ class Session final : private FileIdentReceiver {
bool body_is_compressed = false;
version_type end_version = last_server_version.version;
DownloadCursor download_progress;
UploadCursor upload_progress = {0, 0};
UploadCursor upload_progress = m_upload_threshold;
std::uint_fast64_t downloadable_bytes = 0;
std::size_t num_changesets;
std::size_t accum_original_size;
Expand Down
10 changes: 8 additions & 2 deletions test/object-store/realm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,13 @@ TEST_CASE("Get Realm using Async Open", "[asyncOpen]") {
std::lock_guard<std::mutex> lock(mutex);
return bool(realm_ref);
});
// Write some data
SharedRealm realm = Realm::get_shared_realm(std::move(realm_ref));
realm->begin_transaction();
realm->read_group().get_table("class_object")->create_object_with_primary_key(2);
realm->commit_transaction();
wait_for_upload(*realm);
wait_for_download(*realm);
client_file_id = realm->read_group().get_sync_file_id();
realm->write_copy(config3.path, BinaryData());
}
Expand All @@ -582,7 +588,7 @@ TEST_CASE("Get Realm using Async Open", "[asyncOpen]") {
wait_for_download(*realm);
// Make sure we have got a new client file id
REQUIRE(realm->read_group().get_sync_file_id() != client_file_id);
REQUIRE(realm->read_group().get_table("class_object")->size() == 2);
REQUIRE(realm->read_group().get_table("class_object")->size() == 3);

// Check that we can continue committing to this realm
realm->begin_transaction();
Expand All @@ -593,7 +599,7 @@ TEST_CASE("Get Realm using Async Open", "[asyncOpen]") {
// Check that this change is now in the original realm
wait_for_download(*origin);
origin->refresh();
REQUIRE(origin->read_group().get_table("class_object")->size() == 3);
REQUIRE(origin->read_group().get_table("class_object")->size() == 4);
}

SECTION("downloads Realms which exist on the server") {
Expand Down
96 changes: 96 additions & 0 deletions test/object-store/sync/app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <realm/object-store/sync/mongo_database.hpp>
#include <realm/object-store/sync/mongo_collection.hpp>
#include <realm/object-store/sync/sync_session.hpp>
#include <realm/object-store/thread_safe_reference.hpp>

#include "util/baas_admin_api.hpp"
#include "util/event_loop.hpp"
Expand Down Expand Up @@ -1776,6 +1777,101 @@ TEST_CASE("app: set new embedded object", "[sync][app]") {
}
}

TEST_CASE("app: make distributable client file", "[sync][app]") {
auto config = get_integration_config();
auto base_path = util::make_temp_dir();
util::try_remove_dir_recursive(base_path);
util::try_make_dir(base_path);
util::try_make_dir(base_path + "/orig");
util::try_make_dir(base_path + "/copy");

// Create realm file without client file id
{
TestSyncManager sync_manager(TestSyncManager::Config(config), {});
auto app = sync_manager.app();
app->log_in_with_credentials(AppCredentials::anonymous(),
[&](std::shared_ptr<SyncUser> user, Optional<app::AppError> error) {
REQUIRE(!error);
REQUIRE(user);
});

ThreadSafeReference realm_ref;

realm::Realm::Config realm_config;
realm_config.sync_config = std::make_shared<realm::SyncConfig>(app->current_user(), bson::Bson("foo"));
realm_config.schema_version = 1;
realm_config.path = base_path + "/orig/default.realm";

std::mutex mutex;
auto task = realm::Realm::get_synchronized_realm(realm_config);
task->start([&](ThreadSafeReference ref, std::exception_ptr error) {
std::lock_guard<std::mutex> lock(mutex);
REQUIRE(!error);
realm_ref = std::move(ref);
});
util::EventLoop::main().run_until([&] {
std::lock_guard<std::mutex> lock(mutex);
return bool(realm_ref);
});
SharedRealm realm = Realm::get_shared_realm(std::move(realm_ref));

// Write some data
realm->begin_transaction();
CppContext c;
Object::create(c, realm, "Person",
util::Any(realm::AnyDict{{"_id", util::Any(ObjectId::gen())},
{"age", INT64_C(64)},
{"firstName", std::string("Paul")},
{"lastName", std::string("McCartney")}}));
realm->commit_transaction();
wait_for_upload(*realm);
wait_for_download(*realm);

realm->write_copy(base_path + "/copy/default.realm", BinaryData());

// Write some additional data
realm->begin_transaction();
Object::create(c, realm, "Dog",
util::Any(realm::AnyDict{{"_id", util::Any(ObjectId::gen())},
{"breed", std::string("stabyhoun")},
{"name", std::string("albert")},
{"realm_id", std::string("foo")}}));
realm->commit_transaction();
wait_for_upload(*realm);
}
// Starting a new session based on the copy
{
TestSyncManager sync_manager(TestSyncManager::Config(config), {});
auto app = sync_manager.app();
app->log_in_with_credentials(AppCredentials::anonymous(),
[&](std::shared_ptr<SyncUser> user, Optional<app::AppError> error) {
REQUIRE(!error);
REQUIRE(user);
});

ThreadSafeReference realm_ref;

realm::Realm::Config realm_config;
realm_config.sync_config = std::make_shared<realm::SyncConfig>(app->current_user(), bson::Bson("foo"));
realm_config.schema_version = 1;
realm_config.path = base_path + "/copy/default.realm";

SharedRealm realm = realm::Realm::get_shared_realm(realm_config);
wait_for_download(*realm);

// Check that we can continue committing to this realm
realm->begin_transaction();
CppContext c;
Object::create(c, realm, "Dog",
util::Any(realm::AnyDict{{"_id", util::Any(ObjectId::gen())},
{"breed", std::string("bulldog")},
{"name", std::string("fido")},
{"realm_id", std::string("foo")}}));
realm->commit_transaction();
wait_for_upload(*realm);
}
}

TEST_CASE("app: sync integration", "[sync][app]") {
const auto schema = Schema{{"Dog",
{{"_id", PropertyType::ObjectId | PropertyType::Nullable, true},
Expand Down
5 changes: 5 additions & 0 deletions tools/run_baas_docker_image.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash

PROJECT_DIR=$(git rev-parse --show-toplevel)
MDBREALM_TEST_SERVER_TAG=$(grep MDBREALM_TEST_SERVER_TAG ${PROJECT_DIR}/dependencies.list |cut -f 2 -d=)
docker run --rm -p 9090:9090 -v ~/.aws/credentials:/root/.aws/credentials -it docker.pkg.github.com/realm/ci/mongodb-realm-test-server:${MDBREALM_TEST_SERVER_TAG}