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

Fix client reset cycle detection for PBS recovery errors #7149

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Nov 18, 2023

Tracking that a client reset was in progress was done in the same write transaction as the recovery operation, so if recovery failed the tracking was rolled back too. This worked for FLX due to that codepath committing before beginning recovery.

The good news is that we don't have any reports of users running into this problem, which is a strong sign that client reset recovery failing isn't a common scenario in the first place.

Tracking that a client reset was in progress was done in the same write
transaction as the recovery operation, so if recovery failed the tracking was
rolled back too. This worked for FLX due to that codepath committing before
beginning recovery.
@tgoyne tgoyne self-assigned this Nov 18, 2023
Copy link

Pull Request Test Coverage Report for Build github_pull_request_285034

  • 31 of 31 (100.0%) changed or added relevant lines in 3 files are covered.
  • 672 unchanged lines in 18 files lost coverage.
  • Overall coverage decreased (-0.04%) to 91.677%

Files with Coverage Reduction New Missed Lines %
src/realm/query_engine.hpp 2 94.17%
src/realm/sync/noinst/protocol_codec.hpp 3 76.72%
src/realm/util/file.cpp 3 82.2%
src/realm/unicode.cpp 4 90.15%
src/realm/util/assert.hpp 4 87.1%
src/realm/alloc_slab.cpp 5 92.89%
src/realm/sync/noinst/client_impl_base.cpp 6 85.25%
test/fuzz_group.cpp 8 53.37%
src/realm/sync/noinst/server/server.cpp 16 76.16%
src/realm/index_string.hpp 22 82.86%
Totals Coverage Status
Change from base Build 1847: -0.04%
Covered Lines: 231270
Relevant Lines: 252267

💛 - Coveralls

@tgoyne tgoyne requested a review from ironage November 19, 2023 00:30
Copy link
Contributor

@ironage ironage left a comment

Choose a reason for hiding this comment

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

Good find!

@tgoyne tgoyne merged commit 6b63d4f into master Nov 20, 2023
3 checks passed
@tgoyne tgoyne deleted the tg/pbs-client-reset-cycle branch November 20, 2023 19:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
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 this pull request may close these issues.

2 participants