Skip to content

Enable automatic client reset handling for audit Realms #8072

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

Merged
merged 2 commits into from
Mar 3, 2025

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Feb 26, 2025

Audit Realms typically don't get client resets due to being very short lived, but restarting sync while one is partially uploaded can result in a bad client file ident error which also blocks uploading any other unuploaded files.

Standard automatic recovery is very inefficient here - it redownloads the already uploaded data for the partition, bypasses the optimization to not actually apply the transaction logs locally, and then rewrites the transaction logs - but doing the optimal thing turned out to actually be pretty complicated and I couldn't get it fully working after spending more time on it than is justified.

@tgoyne tgoyne self-assigned this Feb 26, 2025
@cla-bot cla-bot bot added the cla: yes label Feb 26, 2025
Copy link

coveralls-official bot commented Feb 26, 2025

Pull Request Test Coverage Report for Build thomas.goyne_851

Details

  • 77 of 79 (97.47%) changed or added relevant lines in 2 files are covered.
  • 81 unchanged lines in 14 files lost coverage.
  • Overall coverage increased (+0.01%) to 91.132%

Changes Missing Coverage Covered Lines Changed/Added Lines %
test/object-store/audit.cpp 75 77 97.4%
Files with Coverage Reduction New Missed Lines %
src/realm/list.cpp 1 87.37%
src/realm/sync/instructions.hpp 1 76.71%
test/test_index_string.cpp 1 93.48%
src/realm/db.cpp 2 92.19%
src/realm/sync/instruction_applier.cpp 3 68.01%
src/realm/sync/instruction_replication.cpp 3 91.48%
src/realm/sync/noinst/protocol_codec.hpp 3 76.27%
src/realm/sync/noinst/server/server_history.cpp 5 62.59%
test/object-store/util/sync/baas_admin_api.cpp 5 84.57%
test/fuzz_group.cpp 6 51.2%
Totals Coverage Status
Change from base Build 2651: 0.01%
Covered Lines: 217476
Relevant Lines: 238639

💛 - Coveralls

@tgoyne tgoyne force-pushed the tg/audit-client-reset branch from 034621a to f600ba0 Compare February 27, 2025 02:13
@tgoyne tgoyne requested a review from jedelbo February 27, 2025 03:04
Copy link
Contributor

@jedelbo jedelbo left a comment

Choose a reason for hiding this comment

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

LGTM

@tgoyne tgoyne merged commit 4aa15f6 into master Mar 3, 2025
41 of 44 checks passed
@tgoyne tgoyne deleted the tg/audit-client-reset branch March 3, 2025 17:12
@github-actions github-actions bot mentioned this pull request Mar 7, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants