Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put Memory Blob Storage Under a Flag #21612

Merged

Conversation

anthony-murphy
Copy link
Contributor

@anthony-murphy anthony-murphy commented Jun 25, 2024

We recently made changes to how detached blob storage works, and enabled a default in-memory blob storage such that blobs would be supported by default in detached containers.

However, when blobs are in a detached container, it changes the container attachment flow, specifically, an empty file is created, the blobs are uploaded, and then the summary is uploaded. Due to this differing flow some drivers, like odsp, create files with a .tmp extension appended to the file name, this is meant to signify that that file could be incomplete. For example, the summary could fail to upload after blobs have been uploaded, and this will not be a valid fluid file. it is expected that the application will rename the file after attachment completes if the .tmp extension is not desired. If an application does not expect the .tmp extension this can create issues, so for now, we will put this support under a flag, that must be explicitly enabled "Fluid.Container.MemoryBlobStorageEnabled" = true so that apps must opt into this behavior.

related to #21144

@github-actions github-actions bot added area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Jun 25, 2024
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +746 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 451.81 KB 451.84 KB +35 Bytes
azureClient.js 550.23 KB 550.44 KB +216 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 255.73 KB 255.74 KB +14 Bytes
fluidFramework.js 384.62 KB 384.64 KB +14 Bytes
loader.js 133.3 KB 133.47 KB +181 Bytes
map.js 41.1 KB 41.11 KB +7 Bytes
matrix.js 143.38 KB 143.38 KB +7 Bytes
odspClient.js 518.3 KB 518.51 KB +216 Bytes
odspDriver.js 97.21 KB 97.23 KB +21 Bytes
odspPrefetchSnapshot.js 42.21 KB 42.22 KB +14 Bytes
sharedString.js 159.88 KB 159.89 KB +7 Bytes
sharedTree.js 375.08 KB 375.09 KB +7 Bytes
Total Size 3.23 MB 3.23 MB +746 Bytes

Baseline commit: b1a6f32

Generated by 🚫 dangerJS against d64451a

@jason-ha
Copy link
Contributor

If an application does not expect the .tmp extension this can create issues, so for now, we will put this support under a flag, that must be explicitly enabled "Fluid.Container.MemoryBlobStorageEnabled" = true so that apps must opt into this behavior.

Will it be possible to make the default the other way around in a later rc? Can we default to preferred for 2.0? i.e. "Fluid.Container.MemoryBlobStorageEnabled" = false would be needed to disable it?

Or would it be okay to add option to disable and have Loop etc. explicitly disable on pickup?

Also asked in Teams if discussion preferred there.

Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

Let's confirm how we want to ship 2.0. If this is not the preferred 2.0 stance then don't target main. Blocking just to confirm.

Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

Okay to ship 2.x with this defaulting off.

@anthony-murphy anthony-murphy merged commit 76c51ac into microsoft:main Jun 25, 2024
32 checks passed
@anthony-murphy anthony-murphy deleted the memory-blob-storage-flag branch June 25, 2024 21:02
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit to anthony-murphy/FluidFramework-1 that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to microsoft#21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to #21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to #21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to #21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
anthony-murphy added a commit that referenced this pull request Jun 25, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to #21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
rohandubal pushed a commit that referenced this pull request Jun 26, 2024
We recently made changes to how detached blob storage works, and enabled
a default in-memory blob storage such that blobs would be supported by
default in detached containers.

However, when blobs are in a detached container, it changes the
container attachment flow, specifically, an empty file is created, the
blobs are uploaded, and then the summary is uploaded. Due to this
differing flow some drivers, like odsp, create files with a `.tmp`
extension appended to the file name, this is meant to signify that that
file could be incomplete. For example, the summary could fail to upload
after blobs have been uploaded, and this will not be a valid fluid file.
it is expected that the application will rename the file after
attachment completes if the `.tmp` extension is not desired. If an
application does not expect the `.tmp` extension this can create issues,
so for now, we will put this support under a flag, that must be
explicitly enabled `"Fluid.Container.MemoryBlobStorageEnabled" = true`
so that apps must opt into this behavior.

related to #21144

Co-authored-by: Tony Murphy <anthonm@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader 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.

4 participants