Only track pending client resets done by the same core version #7944
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the previous attempt at performing a client reset was done with a different core version then we should retry the client reset as the new version may have fixed a bug that made the previous attempt fail (or may be a downgrade to a version before when the bug was introduced). This also simplifies the tracking as it means that we don't need to be able to read trackers created by different versions. This also means that we can freely change the schema of the table, which this takes advantage of to drop the unused primary key and make the error required, as we never actually stored null and the code reading it would have crashed if it encountered a null error.
tgoyne
force-pushed
the
tg/client-reset-core-version
branch
from
5a1a7b6 to
1815605
Compare
Pull Request Test Coverage Report for Build thomas.goyne_478Details
💛 - Coveralls |
michael-wb
approved these changes
Aug 3, 2024
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the previous attempt at performing a client reset was done with a different core version than the current one, then attempting the client reset again is not yet a cycle as something significant has changed between the two attempts. This means that we don't actually need the ability to read old client reset tracker schemas; if the schema isn't an exact match we can just discard the existing data.
Running with the idea that retrying the client reset if the core version has changed is not only acceptable but desirable, I also made it store the exact core version which wrote the tracker entry and discard it if the version has changed even if the schema hasn't.
Since I was touching the schema for this anyway, I removed the unused primary key and made the error fields required - we never actually used a null error, and the code reading the tracker entry crashed if it encountered one (
obj.get<int64_t>()requires that you check for null first, which the code didn't do).