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

download progress estimate is always 1.0 #7869

Closed
kiburtse opened this issue Jul 8, 2024 · 3 comments · Fixed by #7870
Closed

download progress estimate is always 1.0 #7869

kiburtse opened this issue Jul 8, 2024 · 3 comments · Fixed by #7870
Assignees

Comments

@kiburtse
Copy link
Contributor

kiburtse commented Jul 8, 2024

Expected results

non-zero values reported during sync

Actual Results

estimate is always 1.0

Steps & Code to Reproduce

have some data on the server, open new realm and set the query to fetch some of it, wait for subscriptions to bootstrap, check reported values

sample testcase to see the problem:
TEST_CASE("progress_2") {
    Schema schema{ObjectSchema("TopLevel", {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}},
                                            {"queryable_int_field", PropertyType::Int},
                                            {"payload", PropertyType::String}})};
    FLXSyncTestHarness harness("flx_download_progress", {schema, {"queryable_int_field", "payload"}});

    int num_objects = 9;
    auto add_objects = [&](SharedRealm& realm, const std::string& table_name, int num_objects) {
        auto make_one = [](int64_t idx) {
            return AnyDict{
                {"_id", ObjectId::gen()}, {"queryable_int_field", idx}, {"payload", random_string(1024 * 1024)}};
        };

        CppContext c(realm);
        for (int i = 0; i < num_objects; ++i) {
            realm->begin_transaction();
            Object::create(c, realm, table_name, std::any(make_one(i)));
            realm->commit_transaction();
        }
    };

    auto make_progress_callback = [](const std::string& tag) {
        return [t = tag](uint64_t transferred, uint64_t transferrable, double estimate) {
            cout << t << " | PROGRESS: " << transferred << " -> " << transferrable << ' ' << std::fixed
                 << std::setprecision(2) << estimate << endl;
        };
    };

    cout << "\n1. upload initial data through upload realm..." << endl;
    SyncTestFile upload_config(harness.session().app()->current_user(), harness.schema(), SyncConfig::FLXSyncEnabled{});
    auto upload_realm = Realm::get_shared_realm(upload_config);
    subscribe_to_all_and_bootstrap(*upload_realm);
    add_objects(upload_realm, "TopLevel", num_objects);
    upload_realm->sync_session()->register_progress_notifier(make_progress_callback(">>> upload"),
                                                             SyncSession::ProgressDirection::upload, true);
    wait_for_upload(*upload_realm);

    SyncTestFile config(harness.app()->current_user(), harness.schema(), SyncConfig::FLXSyncEnabled{});
    auto realm = Realm::get_shared_realm(config);

    cout << "\n1.1. add subscription q_1 to download realm and sync..." << endl;

    auto table = realm->read_group().get_table("class_TopLevel");
    auto int_col = table->get_column_key("queryable_int_field");
    int split = num_objects / 3;
    auto q_1 = Query(table).between(int_col, 0, split - 1);

    realm->sync_session()->register_progress_notifier(make_progress_callback("\t\t<<< download streaming cb"),
                                                      SyncSession::ProgressDirection::download, true);
    realm->sync_session()->register_progress_notifier(make_progress_callback("\t\t<<< download non-stream 1"),
                                                      SyncSession::ProgressDirection::download, false);

    {
        auto subs = realm->get_latest_subscription_set().make_mutable_copy();
        subs.insert_or_assign(q_1);
        subs.commit();
    }
    {
        realm->sync_session()->resume();
        realm->get_latest_subscription_set()
            .get_state_change_notification(sync::SubscriptionSet::State::Complete)
            .get();
        realm->refresh();
    }
    cout << "... initial sync is completed." << endl;

    CHECK(q_1.count() == 3);

    cout << "\n2. upload more changes through upload realm..." << endl;
    realm->sync_session()->pause();
    add_objects(upload_realm, "TopLevel", num_objects);
    wait_for_upload(*upload_realm);

    cout << "\n2.1. download new changes..." << endl;
    realm->sync_session()->register_progress_notifier(make_progress_callback("\t\t<<< download non-stream 2"),
                                                      SyncSession::ProgressDirection::download, false);

    realm->sync_session()->resume();
    wait_for_download(*realm);
    realm->sync_session()->pause();
    realm->refresh();

    CHECK(q_1.count() == 6);
}
log output
1. upload initial data through upload realm...
>>> upload | PROGRESS: 6292132 -> 8389486 0.75
>>> upload | PROGRESS: 7340809 -> 9438163 0.78
>>> upload | PROGRESS: 8389486 -> 9438163 0.89
>>> upload | PROGRESS: 9438163 -> 9438163 1.00

1.1. add subscription q_1 to download realm and sync...
                <<< download non-stream 1 | PROGRESS: 1048759 -> 1048759 1.00
                <<< download streaming cb | PROGRESS: 1048759 -> 1048759 1.00
                <<< download streaming cb | PROGRESS: 2097451 -> 2097451 1.00
                <<< download streaming cb | PROGRESS: 3146143 -> 3146143 1.00
... initial sync is completed.

2. upload more changes through upload realm...
>>> upload | PROGRESS: 9438163 -> 10486840 0.00
>>> upload | PROGRESS: 10486840 -> 11535517 0.50
>>> upload | PROGRESS: 11535517 -> 12584194 0.67
>>> upload | PROGRESS: 12584194 -> 13632871 0.75
>>> upload | PROGRESS: 12584194 -> 14681548 0.60
>>> upload | PROGRESS: 13632871 -> 15730225 0.67
>>> upload | PROGRESS: 14681548 -> 16778902 0.71
>>> upload | PROGRESS: 15730225 -> 17827579 0.75
>>> upload | PROGRESS: 16778902 -> 18876256 0.78
>>> upload | PROGRESS: 17827579 -> 18876256 0.89
>>> upload | PROGRESS: 18876256 -> 18876256 1.00

2.1. download new changes...
                <<< download non-stream 2 | PROGRESS: 3146143 -> 3146143 1.00
                <<< download streaming cb | PROGRESS: 4194820 -> 4194820 1.00
                <<< download streaming cb | PROGRESS: 5243497 -> 5243497 1.00
                <<< download streaming cb | PROGRESS: 6292174 -> 6292174 1.00

Core version

Core version: 14.10.3

#7780 and #7796 changed how the progress is processed, seemingly assuming that the progress from download message is stored in history, but for bootstrap messages it's first stored in bootstrap store, until all messages are received, and only then the changesets are applied. This effectively means that check_progress fetches only last pre-bootstrap value for progress.
Also, now estimate is treated as bytes unconditionally even for flx and initial history scan on reconnect, effectively mixing the values.

Copy link

sync-by-unito bot commented Jul 8, 2024

➤ PM Bot commented:

Jira ticket: RCORE-2193

@kiburtse
Copy link
Contributor Author

kiburtse commented Jul 8, 2024

@jbreams @tgoyne what was the idea behind #7780 to make it compatible for multiprocess sync through the file? I wasn't able to review the change since it overlapped with my timeoff, but i don't quite see how these changes made sense. From what i understand, the downloadable bytes or progress estimate from DOWNLOAD message are only relevant during active sync connection (we were essentially emitting every notification only at the time of receiving the message), and only after changeset is stored (for initial bootstrap or change of query) or applied immediately in steady state on reconnect. Saving these values for later didn't provide anything really for download path. In multiprocess sync should it be reused later somehow?

@tgoyne
Copy link
Member

tgoyne commented Jul 8, 2024

The process which is not the one connected to the server needs to be able to report synchronization progress to the developer while it is happening, which requires storing it somewhere that can be shared between processes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants