[FIXED] Make o.Update(state) consistent for file and mem & fixed int underflow
#6147
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
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
derekcollison
approved these changes
Nov 20, 2024
|
|
||
| // Check to see if this is an outdated update. | ||
| if state.Delivered.Consumer < o.state.Delivered.Consumer || state.AckFloor.Stream < o.state.AckFloor.Stream { | ||
| return fmt.Errorf("old update ignored") |
There was a problem hiding this comment.
What effect upstream does this have by changing this to an error where as before it was ignored silently?
There was a problem hiding this comment.
A few cases where this has an effect upstream, most importantly all places already handle/bubble up any error. This is now also more consistent, since it would be done for memstore already but not for filestore before this change.
Also, setStoreState improves in that it doesn't call o.applyState(state) when the o.store.Update(state) was actually ignored/errored. Which was a bug before that would have the states drift.
err := o.store.Update(state)
if err == nil {
o.applyState(state)
}
MauriceVanVeen
added a commit
that referenced
this pull request
Nov 21, 2024
…t underflow (#6147) Made `o.Update(state)` consistent for file and mem stores. For example: mem would not report for certain error conditions and file would not report an error for an error condition. Also fixed integer underflow in `o.checkAckFloor`. Signed-off-by: Maurice van Veen <github@mauricevanveen.com> --------- Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Made
o.Update(state)consistent for file and mem stores. For example: mem would not report for certain error conditions and file would not report an error for an error condition.Also fixed integer underflow in
o.checkAckFloor.Signed-off-by: Maurice van Veen github@mauricevanveen.com