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

How to reconcile TUS Design with Apple Design for Background Uploads? #170

Closed
NickNeedsAName opened this issue Aug 16, 2023 · 23 comments
Closed

Comments

@NickNeedsAName
Copy link

TUS promotes the use of many, tiny uploads so that if any one fails at least all the others still worked and we can pick up where we left off. Love it.

WRT background transfers, Apple promotes the use of fewer, larger tasks. Don't love it, but I understand it.

Is there a way to reconcile these two philosophies?

Specifically:
(My Setup: TUSKit client, tusd server)

  • I begin an upload of a large file (400MB) while my app is in the foreground. Everything is fine, I am seeing the expected logs on both client + server.
  • I then lock my phone.
  • Upload continues for a while as expected
  • After a while (i have not measured, but I would bet it's w/e Apple's "You have to finish your shit after your app is moved out of the foreground after which point we stop letting you start new tasks" timeout is) the logs on my server stop
  • Unlocking my phone/Bringing app to foreground causes the upload to resume (as expected)

The expected behavior would be that the app continues to upload the file in the background while the phone is locked//app is in the background.

Is this the reason for the "chunkSize: 0"? I'm assuming this just says "Do the whole file in one go" but doesn't that completely invalidate the entire point of TUS? What happens if an upload is interrupted due to signal dropping etc.? Do we just assume that Apple is going to revive the session appropriately, or does the tus server handle the partial upload properly?

Apologies if any of this is a duplicate, I can't find any documentation about this anywhere though.

@donnywals
Copy link
Collaborator

Hi Nick!

This is something we can document better for sure. You're correct on Apple's philosophy but not entirely on TUS' small uploads.

To address the point though, let's say you do want to have lots of tiny uploads there is no way to find a good happy medium where we follow Apple's recommendation and a tiny upload strategy. In fact, with the recent work we did to support background uploads this falls apart entirely because starting new uploads while in the background is penalized by Apple.

That brings us to TUS and large file uploads because having a large file transfer is the only way to have good background upload support.

With TUS, the server will keep track of bytes received even in a single very large transfer. If the upload fails mid-transfer, TUSKit will still know the resource identifier that we used to upload to in the first (failed) request. We can ask the server for a status for that existing resource (the SDK handles this currently). The server will then respond with the byte range that it has received for that resource.

The next step is to start a new upload from the first missing byte to the end of the file. The server will append this data to the existing resource with a full upload as a result.

In short, a server that supports TUS handles this correctly.

Thanks for asking this and I've noted down an action item to explain this better in the TUSKit docs.

@NickNeedsAName
Copy link
Author

Thank you for the quick response!! This is what I was expecting to see, but I haven't been able to make it happen.

Right now what I'm doing is

let _ = try tusClient.uploadFileAt(filePath: file, customHeaders: headers)

and then I'm tracking the upload progress via the delegate method

func progressFor(id: UUID, context: [String : String]?, bytesUploaded: Int, totalBytes: Int, client: TUSClient) {
        print("TUS | Progress for \(id): \(bytesUploaded)/\(totalBytes)")
    }

Here's how I setup the client:

tusClient = try TUSClient(server: URL(string:host +tusPath)!,
                                      sessionIdentifier:TUS_SESS_ID,
                                      sessionConfiguration: bgSessCfg,
                                      storageDirectory: URL(string: "/TUS")!,
                                      chunkSize: 300000000)

(nginx is setup to have max client size of 300M, it's arbitrary, I can change it if it matters)

To test network failure I'm just turning off wifi on my phone. When I turn it back on the request starts back over from 0 as indicated by the delegate progress method. I've tried letting it timeout, forcibly closing things, you name it. Do you have any idea what could be the issue or how to force the behavior you described?

The server is https://github.com/tus/tusd + s3store - To be entirely honest, I don't fully understand how the server is supposed to partially process a request that fails? Like on my end what I see is the server just doesn't have a request come in and just doesn't do anything. It's not like it's accepting streaming data and is processing it as it comes in and then goes "oh shit, the request just aborted without finishing or closing, better do TUS stuff" (which is what I'd want to see) it just doesn't do anything at all. Do you have any insight into this side of things?

@donnywals
Copy link
Collaborator

That's interesting, it should work automatically.

I'm not too certain on the server part, maybe @Acconut can help us with that.

For double checking your implementation, the sample app in this repository should work as expected based on my testing. Could you do me a favor and verify that this is the case for you too? Also, which version of TUSKit are you on? Just to be sure that we're looking at the correct version and everything

@Acconut
Copy link
Member

Acconut commented Aug 17, 2023

The server is https://github.com/tus/tusd + s3store - To be entirely honest, I don't fully understand how the server is supposed to partially process a request that fails? Like on my end what I see is the server just doesn't have a request come in and just doesn't do anything. It's not like it's accepting streaming data and is processing it as it comes in and then goes "oh shit, the request just aborted without finishing or closing, better do TUS stuff" (which is what I'd want to see) it just doesn't do anything at all. Do you have any insight into this side of things?

If you are running tusd behind a proxy, you must disable request buffering: https://github.com/tus/tusd/blob/main/docs/faq.md#can-i-run-tusd-behind-a-reverse-proxy Did you do that?

@NickNeedsAName
Copy link
Author

@Acconut - Almost certainly not, that sounds like what I need to do!

@donnywals - I'm going to try the reverse proxy buffering thing, if that doesn't work I'm going to just run the cli tusd + the demo. Version is 3.2.0

Thank you both for your help! and quick responses.

@NickNeedsAName
Copy link
Author

@Acconut - proxy_buffering was at least half of the battle. I'm now seeing tusd chunk write events as the data streams in! As soon as I read what you said, was pretty sure this was the issue.

@donnywals - TUSKit still isn't behaving exactly as I'd expect given the goal (but about as I'd expect given my understanding of URLSession etc.)

I'm pretty sure URLSession.background will attempt to restart the tasks it has in flight when the connection is restored. This is what I think is causing the misbehaviors I'm seeing that I'll describe below. If you have any advice on how to make it not do that for these requests and just cancel them when they fail or w/e (is there a delegate I should be handling? a config setting?)

So the data is being properly streamed in and written. The current read timeout for tusd is 60s. If I turn my wifi off and back on in <60s the upload will completely restart. My assumption here is that because the initial connection doesn't close, the chunk writes are not finalized, so when URLSession restarts the connection it starts over and there's no 'definitive' .info file to check so it continues on with it's merry self. This seems wrong? Ideally I'd like to see TUSKit restart the upload, not URLSession.

In the instance where i leave my wifi off for >60s, i then see "BodyReadError", "ChunkWriteComplete", "ResponseOutgoing" events for tusd. When I turn my wifi back on, the upload fails due to offset mismatch (which is what I would expect) but then it all just fails and I can't restart it (not what I expect). I haven't had time to dig much more into this (maybe I'm not handing HEAD request properly? Something else?) but figured I'd let you know where I'm at.

I also upgraded to iOS 17 last night in an attempt to make all this work since apparently URLSession does a lot of this stuff out of the box now and maybe that's causing a bit of havoc on the TUSKit side?

As always, thanks for your help.

@Acconut
Copy link
Member

Acconut commented Aug 18, 2023

In the instance where i leave my wifi off for >60s, i then see "BodyReadError", "ChunkWriteComplete", "ResponseOutgoing" events for tusd. When I turn my wifi back on, the upload fails due to offset mismatch (which is what I would expect) but then it all just fails and I can't restart it (not what I expect). I haven't had time to dig much more into this (maybe I'm not handing HEAD request properly? Something else?) but figured I'd let you know where I'm at.

This could be an indication that we are indeed missing a HEAD request. Would you be able to look into this, @donnywals?

@donnywals
Copy link
Collaborator

donnywals commented Aug 18, 2023 via email

@Acconut
Copy link
Member

Acconut commented Aug 18, 2023

I haven’t tested iOS 17 yet but I do know that that supports resumable uploads out of the box now and if I understood correctly it does this slightly different from what the SDK would normally do. I will test this too.

This is a great topic that we should discuss soon! I have build an example app using the new API in https://github.com/tus/draft-example/tree/main/clients/ios, and we should look into how we can integrate this into TUSKit

@NickNeedsAName
Copy link
Author

This could be an indication that we are indeed missing a HEAD request. Would you be able to look into this, @donnywals?

I don't think this is the issue. I'm seeing similar behavior with uploads that aren't using TUSKit, just default URLSessionTask stuff. I start the request, I turn off my wifi, turn it back on, and the upload has re-started from 0. I think it's an internal URLSession thing, but I cannot for the life of my figure out if there's any way to control it//stop it. Which is why I'm wondering if you guys have any ideas?

Like what it looks like is happening is I create a BG UploadFile task, I schedule it on the URLSession, and the iOS does a 'best effort' to complete the request. So if the request falters in the middle it just says "That's okay! I'll start it up again!" This new request completely skips the TUS 'HEAD - figure out offset' process, it just reattempts the same request that just cut out for whatever reason. What I'd love to be able to do is tell iOS "hey, this request failed and that's okay! Just let it fail! We can handle the retrying process." And then we do the smart stuff without just blindly retrying.

I suspect this is nontrivial//nonpossible which is why the new stuff is baked into URLSession in iOS17

@NickNeedsAName
Copy link
Author

Like I KNOW that iOS is being stupid when wifi is dropped but cellular is still available. I think a not-uncommon usecase for TUS is that a user starts a decently large upload on WiFi, and then leaves their house. Right now the way iOS is handling things that WiFi request is going to fail, and then the entire upload process is going to be restarted on cellular. Obviously not what we want at all.

@NickNeedsAName
Copy link
Author

NickNeedsAName commented Aug 18, 2023

Also to confirm: This behavior is happening in the default TUSKitDemo. Not sure about the resume, because I'm not sure how to gracefully resume things when iOS is aggressively restarting the request, fucking everything up, and then failing.

The closest I can find is this: https://developer.apple.com/documentation/foundation/urlsessiontaskdelegate/2908819-urlsession

But that's not going to work because waitsForConnectivity is ignored by backgroundSessions "This property is ignored by background sessions, which always wait for connectivity."

The only other thing I can think of is aggressively timing out? But I don't know if that's actually a good idea or not.

@NickNeedsAName
Copy link
Author

Hey guys, any news here or still "no idea what's wrong"?

@NickNeedsAName
Copy link
Author

My gut feeling is that this isn't going to work in any way with a background session (have either of you seen it work with a background session? If so, how?) because of how the background session is controlled. Specifically, according to background session creation docs: "Use this method to initialize a configuration object suitable for transferring data files while the app runs in the background. A session configured with this object hands control of the transfers over to the system, which handles the transfers in a separate process. In iOS, this configuration makes it possible for transfers to continue even when the app itself is suspended or terminated."

So I'm pretttttty sure this is just straight up never going to work. Again, if you've seen it work i'd love to know how!

All I want is a way to swap tasks on the background session in the background and I don't think Apple is going to allow that kind of control :(

@donnywals
Copy link
Collaborator

hey! I've started digging in yesterday but I don't have any good news yet. For "stable" conditions it all works fine. However I'm observing the same thing you are for an intermittent drop of the network connection. I'm trying to figure out to what extent this is something that we can fix, manipulate or work around in some capacity. I'll continue to look and will report back when I have news

@Acconut
Copy link
Member

Acconut commented Nov 22, 2023

@donnywals Hi Donny, were you able to find further insights and possible workarounds for this?

@donnywals
Copy link
Collaborator

donnywals commented Nov 22, 2023

Sadly not. Apple's not providing us with the kind of control that we need to resolve this which is unfortunate. I think it might make sense to go ahead and close this issue since it's not something that's within our control

@Acconut
Copy link
Member

Acconut commented Nov 24, 2023

Alright, that's unfortunate but we gotta live with it :)

Do you think we can improve the documentation on our side to make this limitation clearer?

@donnywals
Copy link
Collaborator

I think that's a reasonable request, our information on background work is somewhat limited currently

@Acconut
Copy link
Member

Acconut commented Nov 24, 2023

I agree, would you be able to extend the current documentation for background handling? :)

@donnywals
Copy link
Collaborator

Of course 👍

@donnywals
Copy link
Collaborator

Closing this issue, documentation has received some updates

@Acconut
Copy link
Member

Acconut commented Dec 5, 2023

Perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants