Skip to content

GC: Tombstone for attachment blob#13266

Merged
agarwal-navin merged 5 commits intomicrosoft:mainfrom
agarwal-navin:tombstoneForAttachmentBlobs
Dec 7, 2022
Merged

GC: Tombstone for attachment blob#13266
agarwal-navin merged 5 commits intomicrosoft:mainfrom
agarwal-navin:tombstoneForAttachmentBlobs

Conversation

@agarwal-navin
Copy link
Copy Markdown
Contributor

@agarwal-navin agarwal-navin commented Dec 6, 2022

Added GC tombstone feature to attachment blobs. Attachment blobs that have been unreferenced for a sufficient amount of time (sweep timeout) become sweep ready, i.e., they can be deleted by GC. With tombstone, these blobs are not deleted but marked as tombstones. Tombstone blobs cannot be retrieved but they are still present and can be recovered.

Some key changes in this PR:

  • BlobManager maintains a list of tombstone blobs. Whenever a blob becomes sweep ready, it is marked as tombstone.
  • In BlobManager::getBlob(), if the blob retrieved is a tombstone, an error is thrown if throwOnTombstone setting is enabled.
  • Moved GC attachment blob tests from test/test-end-to-end-tests/blobs.spec.ts to test/test-end-to-end-tests/gc/gcAttachmentBlobs.spec.ts.
  • Added a bunch of attachement blob tombstone tests.

Note - This is created in favor of #12880.

AB#2132

@agarwal-navin agarwal-navin requested review from a team as code owners December 6, 2022 19:19
@github-actions github-actions Bot added area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc labels Dec 6, 2022
@github-actions github-actions Bot added the base: main PRs targeted against main branch label Dec 6, 2022
@msfluid-bot
Copy link
Copy Markdown
Collaborator

msfluid-bot commented Dec 6, 2022

@fluid-example/bundle-size-tests: +1.04 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 410.57 KB 411.09 KB +531 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 208.02 KB 208.54 KB +530 Bytes
loader.js 154.98 KB 154.98 KB No change
map.js 42.68 KB 42.68 KB No change
matrix.js 133.32 KB 133.32 KB No change
odspDriver.js 120.11 KB 120.11 KB No change
odspPrefetchSnapshot.js 41.2 KB 41.2 KB No change
sharedString.js 152.98 KB 152.98 KB No change
Total Size 1.3 MB 1.3 MB +1.04 KB

Baseline commit: d43b6dc

Generated by 🚫 dangerJS against 1b94256

Comment thread packages/runtime/container-runtime/src/blobManager.ts Outdated
Comment thread packages/runtime/container-runtime/src/blobManager.ts Outdated
Copy link
Copy Markdown
Contributor

@tyler-cai-microsoft tyler-cai-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM I had a few questions, but they were minor.

@tyler-cai-microsoft
Copy link
Copy Markdown
Contributor

#13266 (comment)
This brings a smile to my face :)

@agarwal-navin agarwal-navin merged commit e1c600e into microsoft:main Dec 7, 2022
@agarwal-navin agarwal-navin deleted the tombstoneForAttachmentBlobs branch January 11, 2023 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants