Upload Codepath Refactor

iglesiasbrandon commented 2 years ago

Summary:

We want to Rewrite or refactor the code that performs uploads and downloads inside libuplink to enable a collection of new features and improve upon lessons we've learned from the existing code.

Pain Point:

Our current upload code is rigid and brittle. It has encryption and erasure coding tied too tightly together, and it has hardcoded assumptions about long-tail management and segment pipelining.

Milestone:

https://github.com/storj/uplink/milestone/2

Intended Outcome:

Separate encryption from Reed Solomon so we can build libuplink lite
Add support for dynamic long-tail cancelation
Add better support for parallel upload
Add better support for congestion control and connection establishment
Eliminate stalls between segments

jtolio commented 2 years ago

Notes:

Uploads are not paid and everything about them is already encrypted and signed. We should be able to get away with having uploads be a single signed order and a byte dump without TLS. We should be able to make this way faster.

To do: evaluate what similar opportunities exist for downloads

zeebo commented 2 years ago

Some other high level ideas/goals

It should scale gracefully with the upload capacity of the user. So things like fixed worker pools for uploads and fixed timeouts are probably no good.
It'd be nice if we could avoid full protobuf serialization for the upload messages. We're making many unncessary memory copies. The protobuf wire format for some bytes is very easy to just manually do (send a fixed set of bytes for the field + tag, and then a varint length, and then the data.)
We want to use an interface where you call Write on some object. This helps with C bindings, and matches the current API. Transformations, then, should accept an io.Writer and return an io.Writer, if possible.
There should be some form of back-pressure on the Writes to avoid unbounded memory/disk usage for any scratch space.
We care about separating encryption from rs-encoding so that we can do e2e encrypted gateway-mt. So APIs should exist to allow clients to encrypt streams of bytes without uploading, and upload streams of bytes without encrypting. This is complicated because of the careful coordination required in splitting the segments on appropriate encryption boundaries, issuing begin/commit segment api calls, etc.

Stuff I already did in the past in some branch on my computer somewhere

I have a StreamBuffer type that buffers Writes and lets you grab multiple independent Readers from it
The StreamBuffer has back-pressure built in so that there is bounded write-ahead from the furthest advanced Reader.
The StreamBuffer can be disk or memory backed
There is a StreamSpliiter type that accepts Writes and exports a Next method to get you the next StreamBuffer containing some number of bytes.
The StreamSplitter has minimum and maximum split amounts, so that you can tell if you need to do an inline segment.
I have the beginnings of an API and design idea for how to schedule uploads to estimate upload throughput. The idea is to do a token-bucket sort of thing where the tokens are added based on the minimum time it took to complete a piece upload in the last N uploads. I don't remember all the details about why I thought to do this, but I do remember thinking about it for a really long time.

zeebo commented 2 years ago

I made this project for this .. project: https://github.com/orgs/storj/projects/30/views/1

iglesiasbrandon commented 1 year ago

Note: We started implementing this in Sprint 8 but did some research and design in Sprint 3.

shaupt131 commented 1 year ago

Now expected to be delivered sprint 13. Competing priorities lead to delays in building test automation and finishing QA. Also have a ticket scheduled for sprint 13 to allow it to be enabled but behind a config flag.

shaupt131 commented 1 year ago

Pushing this to a sprint 15 delivery date. We are addressing performance issues related to the refactored upload codepath. Uplink will be reverted to the previous version while we implement improvements.

iglesiasbrandon commented 1 year ago

We are going to move this item to sprint 16. We are having trouble replicating the issue, so we added some monitoring, and those changes are being deployed as a part of v1.80.2 https://github.com/storj/storj/pull/5917

Once these changes are deployed, we hope the extra monitoring will help us uncover and resolve the issue.

iglesiasbrandon commented 1 year ago

The logging we need to help identify the remaining bug was deployed on 6/1. We will monitor this next week and figure out what the issue is so that we can resolve it.

iglesiasbrandon commented 1 year ago

We still dont know what the problem is with the upload code path refactor we need to move this back again.

https://github.com/storj/uplink/issues/147

iglesiasbrandon commented 1 year ago

We have discovered the fixed the issue that will make it so that the new and old upload code paths at least match in performance.

Once that PR is merged, we should consider this issue completed. We also found another limitation that could improve the upload performance even further that we can work on in the future. https://github.com/storj/storj/issues/6020

iglesiasbrandon commented 1 year ago

With the release of uplink v1.11.0, we can call this roadmap item completed!

This was the last bug we were working on in regard to this roadmap item:

https://github.com/storj/uplink/issues/147 https://review.dev.storj.io/c/storj/uplink/+/10730

https://github.com/storj/uplink/releases/tag/v1.11.0

storj / roadmap