SyncServerII / Neebla

Private and Self-Owned Social Media
MIT License
1 stars 2 forks source link

All uploads don't have the same fileGroupUUID #6

Closed crspybits closed 3 years ago

crspybits commented 3 years ago

Dany had an instance of this error from the server recently. I'm reporting in Neebla because this was reported in the client.

2021-02-26 18:52:15.804 [Error] [main] [ServerInterface.swift:101] userEvent(_:event:) > Optional(iOSBasics.ServerAPI.ServerAPIError.non200StatusCode(500))
2021-02-26 18:52:16.112 [Error] [main] [ServerAPI.swift:95] checkForError(statusCode:error:serverResponse:) > nil; serverResponse: Optional("Optional([AnyHashable(\"error\"): Could not FinishUploads: Optional(\"All uploads don\\\'t have the same fileGroupUUID\")])")
2021-02-26 18:52:16.112 [Error] [main] [ServerAPI+FileTransferDelegate.swift:115] uploadCompleted(_:file:response:responseBody:statusCode:) > ServerAPI+FileTransferDelegate.uploadCompleted: non200StatusCode(500)

On the server, the collection of uploads are qualified by: userId, sharingGroupUUID, and deviceUUID.

let fileUploadsResult = params.repos.upload.uploadedFiles(forUserId: currentSignedInUser, sharingGroupUUID: sharingGroupUUID, deviceUUID: deviceUUID, deferredUploadIdNull: true)

https://github.com/SyncServerII/ServerMain/blob/604adddf369a5e8ec077bbfa4bda88a37dcba4c1/Sources/Server/Controllers/FileController/FinishUploadFiles.swift#L79

It seems that in Dany's case, two uploads succeeded, and then one of the uploads failed. There are two rows still existing in the Upload table. I'm not sure what happened after that. Perhaps other uploads were attempted? Presumably it had to be with the same userId, sharing group, and device UUID.

Possible strategies to deal with this:

1) On the client side, a new upload shouldn't be attempted for the same sharing group if one is still pending. This is actually something I didn't have in my mind before. I need to make sure iOSBasics has some constraints that don't allow multiple simultaneous uploads of separate media objects.

2) It seems like there should be some clean up capability server-side. I had Dany resolve the state he found himself in by deleting and re-installing his app. That worked because he got a new device UUID. However, there is still cruft on the server. There are still two rows in the Upload table. And more generally, if a user/client gets into a state like this, they'll be perpetually getting errors. I'm not sure how that clean up would operate. Since there is an error condition on the server when the "All uploads don't have the same fileGroupUUID" is detected, a rollback will be carried out. I'm not sure how I'd do a clean up in that case-- as that will require committing some changes to the database.

3) This also speaks to the need for upload retries. I think I'm not handling that well if at all now.

4) It is possible that there could be ongoing cruft in the Upload table outside of the above. For example, if part of an upload succeeded (at least one file), and then the app crashed and the upload never continued. E.g., if the app was deleted. One strategy to deal with this would be to add dates to the Upload table, and then remove rows from the Upload table that are too old. Not sure when that would be done. Seems a bit overkill to do it on every upload.

crspybits commented 3 years ago

My first strategy is to add a new field to the Uploads table, say "batchUUID"-- this will be an id for the specific N of M batch of uploads.

The goal with this is to ensure I can't get the same problem as I had above. That if one batch of uploads fails somehow that a following batch of uploads can still occur.

I think also that I need to add a batchExpiry field to this table. The purpose of that will be to clean up the Upload table of uploads that are older than certain age.

Each client upload will then need to add these two fields, into the upload request.

crspybits commented 3 years ago

Upload retries: When an upload fails, I need to reset it back to a non started state. Just like I'm doing with downloads right now.

When an upload fails, it's being marked as .notStarted. E.g., https://github.com/SyncServerII/iOSBasics/blob/6f58a46fa33a0e9f76a9a4f7e2ced488d1b1239b/Sources/iOSBasics/SyncServer/SyncServer%2BDelegate/SyncServer%2BDelegateHelpers.swift#L23 Does that mean it will get retriggered for upload later?

In the network URLSession handler, will the appropriate delegate be called?

crspybits commented 3 years ago

Test case:

Upload 1 of 2 files with file group 1, sharing group 1, batch UUID X. (And maybe later 2 of 2).

Upload 1 of 2 files with file group 2, sharing group 1, batch UUID Y. (And maybe later 2 of 2).

In order to have simultaneous uploads of media objects in this same sharing group, this has to work. Not sure if it works now.

This is basically this: https://github.com/SyncServerII/iOSBasics/blob/425c5b5bcf16efb55fe7a548ebe560e071fa296c/Tests/SyncServerTests/UploadQueueTests_TwoObjectDeclarations.swift#L111