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

Offline blob uploads Phase 2 - Support "offline" blob uploads with detached container #6128

Closed
4 tasks done
DLehenbauer opened this issue May 13, 2021 · 11 comments
Closed
4 tasks done
Assignees
Labels
area: runtime Runtime related issues design-required This issue requires design thought epic An issue that is a parent to several other issues feature-request New feature or requests

Comments

@DLehenbauer
Copy link
Contributor

DLehenbauer commented May 13, 2021

This item will track Phase 2 of this effort, comprised of:

--original issue description--
Our recommended path for offline document creation using a detached container does not currently work with SharedTree. This is because the Fluid runtime currently does not support uploading blobs while in a detached state. This is problematic for the SharedTree DDS, which uses Blobs both at the public API layer as well as internally while summarizing.

One potential solution is for the runtime to permit blob uploads while detached, temporarily storing them in memory or persistent storage. During attachment, these blobs will be uploaded to the service prior to sending the initial summary and the IFluidHandle patched with resulting 'absoluteUri'. (For correctness, absoluteUri should be made opaque.)

@ghost ghost added the triage label May 13, 2021
@DLehenbauer DLehenbauer added area: runtime Runtime related issues design-required This issue requires design thought feature-request New feature or requests labels May 13, 2021
@anthony-murphy
Copy link
Contributor

can someone expand the scenarios here? is it just detached? what about serialize? same session only? different sessions?

@coclauso
Copy link
Contributor

Hi @anthony-murphy, thanks for looking at this. Here's more information about this scenario (along with some thinking of what a solution might look like)--hopefully it will address some of the questions.

The scenario we'd like to support is as follows--the host allows editing a detached container, and relies on ExternalContainer.serialize() along with some other storage option of its choice (e.g. IndexedDB) for persistence. At some point in the future, the host may choose to attach the container to a driver, assumed here odsp-driver.

The following issues currently exist with blobs:

  1. When detached, runtime.uploadBlob() doesn't work, and throws an exception.
  2. Assuming this problem were fixed and runtime.uploadBlob() stored the blobs locally, the blob handles returned would end up in ops (in the serialized container), but wouldn't refer to data on Sharepoint. This would create a problem when the container was later attached to odsp-driver (i.e., the blob handles would be meaningless for other clients that don't have or know about the local blobs).

On point (2)--when we attach later to odsp-driver, we'd like to upload all the blobs to Sharepoint and then replace the old handles in the ops with the new Sharepoint handles, but there's some question around the best way to do that.

Here's a sketch of a solution that I've proposed elsewhere--at some point when the host creates the detached container, it provides an instance of an interface (maybe IDetachedBlobStore) with the following methods:

  • storeBlob() -- stores blob of data, returns a (serializable, string or similar) handle
  • retrieveBlobForHandle() -- given handle, returns blob
  • listBlobHandles() -- returns a list of all handles for all stored blobs

If the container is in a detached state, then the Fluid runtime uses storeBlob() and retrieveBlobForHandle() as listed above to implement runtime.uploadBlob() and IFluidHandle.get().

When the container is attached, listBlobHandles() is used to upload all the blobs to Sharepoint. At this point, the runtime has a correspondence map from the old, local handles to the new Sharepoint handles. So the question at this point is how to map old handles in ops to the new handles.

Here's one idea I've floated--the runtime somehow attaches the map with the container data in the driver, either as an op or using the container quorum feature, the outcome is that any client attached to the container can get the map. We assume that every DDS that stores handles in ops will deserialize them to an IFluidHandle via IFluidSerializer.parse(). The idea is that under the hood, if the container is attached, IFluidSerializer.parse() checks if the handle is a local handle, and uses the knowledge of the mapping to translate to a Sharepoint-meaningful IFluidHandle.

Anyways, thanks again for looking at this, and please let me know if I can clarify the scenario further. The approaches above are just to help sketch out what we're hoping for (i.e. as an API consumer), I'm definitely assuming your thinking on how to go about doing it is likely better than mine, here.

Thanks again!

@coclauso
Copy link
Contributor

coclauso commented May 20, 2021

Also--another couple of quick comments:

  1. Currently it's a pattern with odsp-driver for new container instances to allow the user to edit the container locally in a detached state before it's sync'ed to odsp. This means that if the user does anything in this window that triggers blob upload, currently it would cause a failure. Am imagining that a solution to the above problem could be leveraged to cover this edge case.
  2. The high level scenario:

the host allows editing a detached container, and relies on ExternalContainer.serialize() along with some other storage option of its choice (e.g. IndexedDB) for persistence

might also be addressed by allowing a container to be detached from one driver and reattached to another one, however I believe the problem of moving the blobs will still be essentially the same as that described here, using ExternalContainer.serialize().

Thanks again so much.

@vladsud
Copy link
Contributor

vladsud commented Jun 3, 2021

There are multiple directions we can go here, it would be great to first understand constraints.

Some number of questions:

  1. The timeline, to dogfood and to production. Note that any server changes (if they are required) take a long time to production, so we likely need to look into both long term strategy here as well as short term solution we can achieve in given time frame.
  2. User experience while file is not fully uploaded. With big sizes we can't rely on single roundtrip to upload 400Mb of data, and there is no way to get streaming API stood up for file creation in this short time frame, so we should rely on multiple uploads and presence of the file in some state during this time, as well as possibility of client crashing / being closed. There are couple questions that come here:
    • What should be user experience if user attempts to open such partially uploaded file?
    • Do we want to give it some non-fluid extension and rename at the end of the process not to allow user to open it with Whiteboard app?
  3. We would need to have some basic understanding on distribution of blob counts, sizes, and type of data (binary vs. unicode) to better understand pros/cons of various solutions.

As for workflow, and possible designs, here are some thoughts:

  1. A file is created, either zero-byte file, or with some initial (empty) summary (to make it valid fluid file).
    • Need to double check if it is Ok re final summary upload and no ops between initial summary (or empty file) and final summary, i.e. both summaries having same reference sequence number. We really do not want to involve ordering service connection here.
  2. A number of binary blobs are uploaded, one by one sequentially (ODSP does not support concurrent uploads). These are called "attachment" blobs.
    • this is the most efficient process to upload bits for binary large payloads, and very inefficient for small blobs.
    • attachment blobs are not included into TreesLatest (aka snapshot) response when client loads this file later - they are loaded by client on demand.
  3. An incremental summary upload can be used (see issue ODSP: Implement incremental summary upload  #5690) to upload number of blobs in aggregate, i.e. many blobs per request
    • this is efficient for small blobs, especially if they are not binary, i.e. do not need base64 encoding.
    • blobs uploaded this way would be included into TreesLatest (aka snapshot) future response. This might be not desirable, and it's possible to implement new ODSP feature to mark some of these blobs to be treated as "attachment" blobs.
  4. Ability to create attachment blobs (and their handles) in detached container state.
    • Current design assumes that once container starts file creation process, container switched to "attaching" state and all DDSs start to create ops in response to local changes. This means blob handles need to be able to serialize (and thus have blob IDs) through attachment process, before actual blob upload was completed. Some redirection schema is possible to avoid this problem (similar to how it is described above).
    • Once attachment process starts, new blob creation would need to wait till file is created (but not wait for whole creation sequence to finish, i.e. final snapshot upload).
  5. Once the process is complete, a final summary is uploaded, and file is renamed to have .fluid extension (if needed, depending UX question / answer above).

The work on runtime side would include
I'd propose we start simple first, i.e. leverage as much as possible existing flows.

  1. A file is created with some initial (empty) summary.
  2. Upload every attachment blobs directly, not trying to upload small attachment blobs leveraging summary blob creation path (that allows to upload a lot of blobs in one http request). This means process will take a lot of roundtrips (assuming a lot of attachment blobs). Long term, we can create multiple blob upload / download API and leverage that to gain efficiency, instead of leveraging summary blob path.
  3. Full creation process would need to either grab the summary at the start of the process and upload it after all attachment blobs were uploaded. Or alternatively, snap summary at the end, but blob upload process should assume that new blobs can show up blob upload process is considered finished if all blobs create before and during this process are uploaded. Latter process allows us not to generate indirection when creating blob URIs/handles, as handles should not be serialized before first summary is snapped (and container switches to attaching state), and this it's likely preferred approach. Note that GC process does not delete blobs right away, so there is no problem here.

I'm sure I'm missing some key things here, but worth writing it out as is - I'll keep adding to this write up.
Overall - it's a lot of work no matter how we look into it.

@vladsud
Copy link
Contributor

vladsud commented Jun 6, 2021

@coclauso, @DLehenbauer, I've updated my long post above. It would be great for you to take a look and comment, especially on perf aspect of it - I think it would be the most challenging part of whole effort. We could run some experiment with some workloads approximating what users would experience RE uploading that much data and that many blobs in one go, to get a scenes if current approach will work, or not.

Number of roundtrips likely can be addressed in reasonable amount of time by adding multi-blob upload API or leveraging summary blob upload path. But there is really no solution for bandwidth here, and we can't leverage multiple parallel uploads (as SPO can't handle it), so we are limited to what single connection can give us. Note that existing file flow already serializes all attachment uploads, so you can experience it in existing product by ensuring you create scenario that hits many uploads at once.

If the whole process and perf looks reasonable, we could start breaking it into individual work items.

@coclauso
Copy link
Contributor

coclauso commented Jun 7, 2021

Hi @vladsud ,

Thanks for your response. I’ll try to go through this and add my own comments, for whatever light they can shed. Please point out anything I don’t address well. It’s also possible that @DLehenbauer can reply to some portions.

The timeline, to dogfood and to production

I’ll start a separate communication for this with folks who can provide more details.

User experience while file is not fully uploaded

What should be user experience if user attempts to open such partially uploaded file?

Do we want to give it some non-fluid extension and rename at the end of the process not to allow user to open it with the app?

I could imagine something like .[extension].partial or .[extension].incomplete until it’s fully uploaded. On our side, we’re planning to delete the local data once it’s been transferred to Sharepoint. We should make it such that in the case where the app crashes mid-upload, then that wouldn’t happen, and the next time the app was launched, the upload would retry.

We would need to have some basic understanding on distribution of blob counts, sizes, and type of data (binary vs. unicode) to better understand pros/cons of various solutions.

This might be partially addressed by a comment that follows. In our application, we reserve blobbing for large chunks of data. I believe they’re binary (i.e. we don’t base64 encode) but could double check.

As for workflow, and possible designs, here are some thoughts:

Most of this seems fine, I’d comment on this, though:

An incremental summary upload can be used (see issue #5690) to upload number of blobs in aggregate, i.e. many blobs per request

Right now, we’re using blobs to refer to data that should be stored as Sharepoint attachment, and our application has heuristic code to only blob large chunks of data, not small chunks. My suggestion here would be for simplicity not to consider the case of a “small blob” at the runtime level and leave it to the API consumer to ensure that only large blocks of data are blobbed.

Once attachment process starts, new blob creation would need to wait till file is created (but not wait for whole creation sequence to finish, i.e. final snapshot upload).

I think the question here is, while the document is being transferred to Sharepoint, is it acceptable for some app functionality to block until the transfer is complete? If we decide it is, for simplicity of the API consumer, I'm wondering if the best way to expose this to the app would be to have the async runtime.uploadBlob() function block until the attachment process is complete?

Full creation process would need to either grab the summary at the start of the process and upload it after all attachment blobs were uploaded.

If allowing blob creation during the attachment process rather than blocking isn’t too much harder, it might be worth doing. It could provide a smoother user experience during transfer.

Let me know if there's anything else I can provide that can help.

Thanks,
Cooper

@coclauso
Copy link
Contributor

coclauso commented Jun 7, 2021

Also, on this:

Number of roundtrips likely can be addressed in reasonable amount of time by adding multi-blob upload API or leveraging summary blob upload path. But there is really no solution for bandwidth here, and we can't leverage multiple parallel uploads (as SPO can't handle it)

It's presently the case that attaching a container to Sharepoint takes a number of seconds (approx. 5 or so), generally apps allow the user to edit the document seamlessly during this time so the experience is still good. If this fundamental SPO bandwidth bottleneck were just manifested as a longer attachment waiting time, say a few seconds more, then I'd imagine the existing mechanisms that mask this from the user would be fine here.

This issue relates back to these earlier comments:

I think the question here is, while the document is being transferred to Sharepoint, is it acceptable for some app functionality to block until the transfer is complete?

If allowing blob creation during the attachment process rather than blocking isn’t too much harder, it might be worth doing. It could provide a smoother user experience during transfer.

It could be the right approach here is to allow for the app to create additional blobs during attachment.

@vladsud
Copy link
Contributor

vladsud commented Jun 7, 2021

Thanks! So here is modified plan:

Pre-work:
0. Modify upload blob API to work in detached mode by

  • holding data to be uploaded
  • Issuing handles that go through redirection (i.e. in the form of /_blobs/<guid>, where guid is the unique local ID of the blob, not current blob ID from service). This is required for serialization of detached container. Back compat needs to be considered with existing format.
  • Implement detached serialization & rehydration of binary attached blobs (they would need to be base64 encoded if using current approach, see more below).

Upload process:

  1. Container transitions to Attaching state.
  2. A file is created
    • It's zero byte file. This is natively supported by runtime/loader/driver, i.e. if there are no attachment blobs to upload, then we leverage existing workflow of creation from summary, otherwise we use this workflow.
    • It has some non-fluid extension. FF supports this flow, but app controls naming / renaming.
  3. Upload every attachment blob, sequentially.
    • this process is done when there are no non-uploaded blobs. This includes any new blobs that showed up while # 1-3 was in flight.
    • Making local changes while upload is happening is supported, however they will not be reflected in history of the file (as ops), as snapshot is grabbed at the very end.
  4. Summary is created, and uploaded to file.
  5. When summary upload is is over
    • Container transitions to Attached state.
    • File is renamed to have .fluid extension (performed by app).

Detached serialization / rehydration is easy and small part of it (see prep work), however size should be considered (i.e. is it Ok to serialize all the content of container into single string). Even if it works, it would likely be super-inefficient (too many memory copies, base64 encoding). Future plan likely needs to involve something more complicated, where serialization is done through some interface that supports working with individual binary blobs (this should be considered, but handled outside of this item as future improvement).

We should likely fork this item into two

  1. Prep work and support of detached serialization / deserialization. This is higher priority and needs to be done sooner
  2. The actual 5 steps above to convert it to a file

@coclauso
Copy link
Contributor

coclauso commented Jun 7, 2021

Detached serialization / rehydration is easy and small part of it (see prep work), however size should be considered (i.e. is it Ok to serialize all the content of container into single string). Even if it works, it would likely be super-inefficient (too many memory copies, base64 encoding). Future plan likely needs to involve something more complicated, where serialization is done through some interface that supports working with individual binary blobs (this should be considered, but handled outside of this item as future improvement).

It sounds like we're deciding to boot implementing this suggestion from earlier:

Here's a sketch of a solution that I've proposed elsewhere--at some point when the host creates the detached container, it provides an instance of an interface (maybe IDetachedBlobStore) with the following methods:...

A comment I would make is that if this isn't too difficult, it might be worth doing earlier, the reason is that if a product ships at any point with blobs in the serialized snapshot, then whatever functionality is required to deserialize them from this source has to be maintained indefinitely into the future, so there could be some value in doing this in the first version.

@vladsud
Copy link
Contributor

vladsud commented Jun 7, 2021

Agree, though I'd point out we are already sort of in that bucket (if anybody uses string-based functionality, then their drafts would either be lost, or we would need to build an adapter to convert it to this new format).
Clearly addressing it sooner is better :)
I'd say that we can go with prototype (hidden under experimental / deprecated façade) that uses current string approach to get feedback about overall process (and find first set of bugs), and keep working on moving toward something like IDetachedBlobStore

That said, I'd modify it to have only 4 methods (blob-related):

  • read(blobid) -> content
  • write(content) -> blobId.
  • readSummary()
  • writeSummary()

No listing, no handles.
Maybe add delete() for future (if we ever implement GC for detached state, host can delete everything related to a file on successful file creation).

@danielroney danielroney added the epic An issue that is a parent to several other issues label Jun 14, 2021
@DLehenbauer
Copy link
Contributor Author

@danielroney - FYI: Breakdown of work items below.

Phase1: Support blobs while detached

  • Define IDetachedBlobStorage with create/read methods
  • Plumb IDetachedBlobStorage from host to ContainerStorageAdapter
  • Use IDetachedBlobStorage to persist create/read blobs when detached.
  • Tests (incl. mock implementation of Node.js)

Phase 2: Upload blobs on attachment

  • Change ODSP workflow when attaching with blobs
    • Create 0B file with non-.fluid extension
    • Process queue of pending blob uploads
    • Create/upload summary
    • Rename file to .fluid extension
  • Tests for new creation flow
  • Implement redirection for detached blob URLs
    • Implement hook for uploading blobs on Container.attach()
    • As detached blobs upload, add their before/after URLs to a table and reclaim detached storage.
    • Use the table to resolve requests for "before" URLs.
    • Add snapshot/load APIs to IDetachedBlobStorage
    • Include blob redirection table in summary.
  • Tests for Blob redirection

Open issues:

  • Figure out the appropriate layer to perform uploading of detached blobs and triggering the alternate ODSP creation flow
  • Lower priority, but we should consider if detached blobs should be deduped and/or GC'ed.

@danielroney danielroney changed the title Support "offline" blob uploads with detached container Offline blob uploads Phase 2 - Support "offline" blob uploads with detached container Jun 22, 2021
@danielroney danielroney modified the milestones: June 2021, July 2021 Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues design-required This issue requires design thought epic An issue that is a parent to several other issues feature-request New feature or requests
Projects
None yet
Development

No branches or pull requests

6 participants