feat(counter): Add rollback support for SharedCounter#25771
Merged
Conversation
github-actions
Bot
added
area: dds
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds rollback support to the SharedCounter DDS to enable undoing optimistic operations when a transaction is rolled back.
Key changes:
- Implements rollback functionality in SharedCounter by tracking pending operations
- Adds comprehensive test coverage for rollback scenarios including multiple ops, partial rollbacks, and cross-client interactions
- Enables rollback testing in fuzz tests by removing the rollbackProbability: 0 override
- Integrates the counter model into local-server-stress-tests for broader stress testing
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dds/counter/src/counter.ts | Implements rollback support by tracking pending ops, modifying processMessage to handle local ops optimistically, and adding rollback method |
| packages/dds/counter/src/test/counter.spec.ts | Adds comprehensive rollback test suite with 5 test cases covering various scenarios |
| packages/dds/counter/src/test/counter.fuzz.spec.ts | Enables rollback in fuzz tests by removing rollbackProbability: 0 override |
| packages/test/local-server-stress-tests/package.json | Adds counter package dependency |
| packages/test/local-server-stress-tests/src/ddsModels.ts | Imports and registers baseCounterModel for stress testing |
| pnpm-lock.yaml | Updates lockfile with new counter dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
ChumpChief
approved these changes
Oct 30, 2025
Contributor
ChumpChief
left a comment
ChumpChief
left a comment
There was a problem hiding this comment.
Seems good! One suggestion but probably fine either way
Comment on lines
+159
to
+163
| assert( | ||
| pendingOp !== undefined && | ||
| pendingOp.type === op.type && | ||
| pendingOp.incrementAmount === op.incrementAmount, | ||
| "local op mismatch", |
Contributor
There was a problem hiding this comment.
Consider calling submitLocalMessage with some unique localOpMetadata that is more-definitively matchable, rather than comparing the properties. Maybe just an incrementing number, or even the op itself and you could do object identity comparison.
Mostly just because normal use cases might increment by the same value (esp. +1) on every call and would otherwise be indistinguishable.
ChumpChief
approved these changes
Oct 31, 2025
anthony-murphy-agent
pushed a commit
to anthony-murphy-agent/FluidFramework
that referenced
this pull request
Jan 14, 2026
## Description This PR adds rollback support for the `SharedCounter` DDS. ## Implementation `SharedCounter` only has one type of op (`increment`), which will be reflected optimistically by incrementing the counter's value. To rollback we can simply call `incrementCore()` with opposite of the value that it was optimistically incremented with. This will also emit an `incremented` event with the amount that was rolled back and the new value of the counter. This PR also adds additional bookkeeping for pending ops to ensure we handle local ops properly. This may not be technically necessary but can help catch bugs in the future and is more in line with how other DDSes handle local ops. ## Testing This PR adds the following testing: - UTs for Counter rollback scenarios - Enables rollback for existing Counter fuzz tests - Counter is now used in local server stress tests ## Misc [AB#44705](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/44705)
This was referenced May 27, 2026
This file contains hidden or 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
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.
Description
This PR adds rollback support for the
SharedCounterDDS.Implementation
SharedCounteronly has one type of op (increment), which will be reflected optimistically by incrementing the counter's value. To rollback we can simply callincrementCore()with opposite of the value that it was optimistically incremented with. This will also emit anincrementedevent with the amount that was rolled back and the new value of the counter.This PR also adds additional bookkeeping for pending ops to ensure we handle local ops properly. This may not be technically necessary but can help catch bugs in the future and is more in line with how other DDSes handle local ops.
Testing
This PR adds the following testing:
Misc
AB#44705