Blob placeholders prototype #24158
Draft
Blob placeholders prototype #24158
Conversation
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
This is just for testing purposes. We intentionally make it very hard to observe local object creation, and attach state, in a broad way. Test like the local server stress work around this by storing absolute paths, which allow remote clients to find objects one they are attached but avoid attaching them at that point. Clients walk the list of absolute paths, and find those they can resolve, and then use those in dds operations, this can lead to attach, multiple handles to the same object, and reviving unreferenced objects.
github-actions
bot
added
area: examples
…detached container flow (#24179)
github-actions
bot
removed
public api change
packages/runtime/container-runtime/src/blobManager/blobManager.ts
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/blobManager/blobManager.ts
Outdated
Show resolved
Hide resolved
|
Is there any design doc, one pager or context to help review this approach? I believe I get the general idea through spare conversations but a written starting point would be great and I'd appreciate a pointer in case it already exists. |
github-actions
bot
added
the
area: dds
DocumentSchema is a more appropriate tool than a feature flag, because once it is enabled for a document it permanently puts older incompatible clients at risk of encountering op patterns they aren't prepared for (namely, an op containing the handle before the blob attach op). This PR primarily switches to that mechanism, but it also revealed that I had a bug in my flag enablement in blobManager.spec.ts that was causing the tests to run only in legacy mode (I wasn't paying attention to how/when injectedSettings got wiped out). After fixing that test bug, the now-functioning tests revealed a couple other small bugs: 1. The pendingBlobs created by the placeholder handles should be marked attached immediately upon creation, because they are created by the act of attaching the handle. 2. waitForBlob in the test had an inverted condition - it should be waiting to see its blob appear in the unprocessedBlobs (exists in main but must not be causing problems there) 3. An 0x38f assert in one of the tests that I haven't debugged fully yet, though it may just be a test bug due to the handle management. Because blobManager.spec.ts expects to have such intimate knowledge of the BlobManager internals, it might be reasonable to consider a broader rework of its test coverage for placeholder blobs (since it subverts the expectations about handle and upload behavior). Maybe for later consideration. [AB#34418](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/34418) [AB#34459](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/34459)
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Primary motivation for this change is to ensure customers can still detect upload failures (so there's no feature take-back as compared to the non-placeholder case). Main changes are: * Adds the interfaces to core-interfaces as legacy/alpha * Adds a type check to runtime-utils as legacy/alpha * Update BlobManager/BlobHandle to implement * Add/update two tests for the local/failed/shared states and their transitions (placeholder state is unused until we do remote handles too) * Small rename to be more normally readable (IFluidHandleInternalPlaceholder -> IFluidPlaceholderHandleInternal) [AB#34918](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/34918)
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.
Prototype branch for blob placeholders. Replaces #24137 which has more context.