Skip to content

Offline: Add recent batch info from DuplicateBatchDetector to summary#22454

Merged
markfields merged 17 commits into
microsoft:mainfrom
markfields:o3/duplicates-summary
Nov 18, 2024
Merged

Offline: Add recent batch info from DuplicateBatchDetector to summary#22454
markfields merged 17 commits into
microsoft:mainfrom
markfields:o3/duplicates-summary

Conversation

@markfields
Copy link
Copy Markdown
Member

@markfields markfields commented Sep 9, 2024

Description

Fixes AB#13963

We've added a new data structure for tracking "recent" (based on MSN) batchIds which are at risk for seeing a duplicate sequence batchID come in. This data structure is consulted and modified on each incoming batch.

Whenever we summarize, we need to include this state as of the given sequence number, and when we load from a snapshot we need to repopulate it as well.

Why? Because summaries can happen at any time, and a future container loading from that point will need to know the recent batch IDs that may possibly be followed by a sequenced duplicate.

@github-actions github-actions Bot added area: runtime Runtime related issues public api change Changes to a public API base: main PRs targeted against main branch labels Sep 9, 2024
@markfields markfields marked this pull request as ready for review September 12, 2024 20:02
@markfields markfields requested a review from a team as a code owner September 12, 2024 20:02
@markfields markfields marked this pull request as draft September 20, 2024 18:00
@msfluid-bot
Copy link
Copy Markdown
Collaborator

msfluid-bot commented Sep 24, 2024

@fluid-example/bundle-size-tests: +2.02 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 465.94 KB 466.42 KB +492 Bytes
azureClient.js 562.76 KB 563.25 KB +506 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 262.1 KB 262.56 KB +468 Bytes
fluidFramework.js 427.12 KB 427.13 KB +14 Bytes
loader.js 134.18 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 150.15 KB 150.16 KB +7 Bytes
odspClient.js 528.59 KB 529.09 KB +506 Bytes
odspDriver.js 97.88 KB 97.9 KB +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 166.23 KB 166.24 KB +7 Bytes
sharedTree.js 417.58 KB 417.58 KB +7 Bytes
Total Size 3.37 MB 3.37 MB +2.02 KB

Baseline commit: ac5883c

Generated by 🚫 dangerJS against 076fe47

@markfields markfields requested review from a team, MarioJGMsoft, jatgarg, kian-thompson, pragya91, rajatch5 and tyler-cai-microsoft and removed request for a team October 9, 2024 22:26
@markfields
Copy link
Copy Markdown
Member Author

@kian-thompson can you please take a look? This can't be merged until we're allowed to make breaking changes (it updates the constructor signature of ContainerRuntime). But I'd like to have it ready to go.

Comment thread packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts Outdated
@markfields markfields changed the title Offline: Add recent batch info from DuplicateBatchDetector to summary [BREAKING] Offline: Add recent batch info from DuplicateBatchDetector to summary Oct 10, 2024
markfields and others added 2 commits November 15, 2024 11:43
Co-authored-by: Kian Thompson <102998837+kian-thompson@users.noreply.github.com>
@markfields markfields changed the title [BREAKING] Offline: Add recent batch info from DuplicateBatchDetector to summary Offline: Add recent batch info from DuplicateBatchDetector to summary Nov 15, 2024
@markfields markfields marked this pull request as ready for review November 15, 2024 19:52
Copy link
Copy Markdown
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

↑ packages.runtime.container-runtime.src.summary:
Line Coverage Change: 0.01%    Branch Coverage Change: 0.01%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 95.82% 95.83% ↑ 0.01%
Line Coverage 84.84% 84.85% ↑ 0.01%
↑ packages.runtime.container-runtime.src:
Line Coverage Change: 0.01%    Branch Coverage Change: 0.02%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 92.90% 92.92% ↑ 0.02%
Line Coverage 94.55% 94.56% ↑ 0.01%
↑ packages.runtime.container-runtime.src.opLifecycle:
Line Coverage Change: 0.07%    Branch Coverage Change: 0.11%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 95.23% 95.34% ↑ 0.11%
Line Coverage 95.39% 95.46% ↑ 0.07%

Baseline commit: 74cb3f7
Baseline build: 307922
Happy Coding!!

Code coverage comparison check passed!!

@github-actions
Copy link
Copy Markdown
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  171745 links
    1640 destination URLs
    1840 URLs ignored
       0 warnings
       0 errors


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: runtime Runtime related issues base: main PRs targeted against main branch public api change Changes to a public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants