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

Timeout with CompleteMultipartUpload #30

Closed
Tracked by #50
lperlaki opened this issue Apr 11, 2023 · 6 comments · Fixed by #141
Closed
Tracked by #50

Timeout with CompleteMultipartUpload #30

lperlaki opened this issue Apr 11, 2023 · 6 comments · Fixed by #141
Labels
blocking Unactionable due to upstream issues or design problems feature New feature or request

Comments

@lperlaki
Copy link
Contributor

lperlaki commented Apr 11, 2023

in my use case, a CompleteMultipartUpload can take a longer time and some clients timeout before the s3 server respond.

the Amazon S3 services deals with that problem by sending the response Header early and a whitespace character on an interval until the operation finishes, sending the final response or error as the xml body.

https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html

Processing of a Complete Multipart Upload request could take several minutes to complete. After Amazon S3 begins processing the request, it sends an HTTP response header that specifies a 200 OK response. While processing is in progress, Amazon S3 periodically sends white space characters to keep the connection from timing out. A request could fail after the initial 200 OK response has been sent. This means that a 200 OK response can contain either a success or an error. If you call the S3 API directly, make sure to design your application to parse the contents of the response and handle it appropriately. If you use AWS SDKs, SDKs handle this condition. The SDKs detect the embedded error and apply error handling per your configuration settings (including automatically retrying the request as appropriate). If the condition persists, the SDKs throws an exception (or, for the SDKs that don't use exceptions, they return the error).

could we implement a similar solution? I experimented with it by editing the generated files in a branch(#31) and it works with the client but the solution is not ideal

https://github.com/lperlaki/s3s/tree/complete_multipart_keepalive

@Nugine
Copy link
Owner

Nugine commented Apr 11, 2023

could we implement a similar solution?

Yes. s3s should implement this.

Currently s3s converts the s3 output to a http response in a way of "oneshot". To support the "keepalive" feature, we may need special handling in the codegen.

For example

  • add a stream type representing the "keepalive" behavior
  • treat CompleteMultipartUpload as a streaming operation
  • generate necessary imports from hand-written modules

@Nugine Nugine added the feature New feature or request label Apr 11, 2023
@lperlaki
Copy link
Contributor Author

lperlaki commented Apr 11, 2023

I implemented the behaviour with a custom Body type
one problem I ran into was that hyper expects the the lifetime of the HttpBoby to be static, but the future returned from the Operation has as lifetime tied to the dyn S3 object.

@Nugine
Copy link
Owner

Nugine commented Apr 11, 2023

Question: Where should the whitespaces be inserted?

We won't know the http headers until CompleteMultipartUpload finishes. Does hyper support this usage?

HTTP/1.1 200 OK

{{sending whitespaces to keep alive}}

{{headers}}
{{xml body}}

@Nugine
Copy link
Owner

Nugine commented Apr 11, 2023

one problem I ran into was that hyper expects the the lifetime of the HttpBoby to be static, but the future returned from the Operation has as lifetime tied to the dyn S3 object.

It's OK to change to Arc<dyn S3>.

@lperlaki
Copy link
Contributor Author

lperlaki commented Apr 11, 2023

Question: Where should the whitespaces be inserted?

We won't know the http headers until CompleteMultipartUpload finishes. Does hyper support this usage?

HTTP/1.1 200 OK

{{sending whitespaces to keep alive}}

{{headers}}
{{xml body}}

right now I am sending the headers in the trailer

HTTP/1.1 200 OK
{{headers known ahead of time}}
{{xml declaration}}

{{keep alive whitespace}}

{{xml body}}
{{headers}}

in my testing some client xml parsing libraries expect there to be no whitespace before the xml declaration that is why I am sending it early.

@Nugine Nugine added the blocking Unactionable due to upstream issues or design problems label Apr 21, 2023
@Nugine Nugine mentioned this issue Jun 20, 2023
8 tasks
@lperlaki
Copy link
Contributor Author

The blocking hyper issue hyper#2719 has been closed with hyper#3375 and released with [email protected].
so this issue is now blocked on #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking Unactionable due to upstream issues or design problems feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants