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

Concurrent uploads to S3 fails #491

Closed
josephrony277 opened this issue Oct 10, 2023 · 7 comments
Closed

Concurrent uploads to S3 fails #491

josephrony277 opened this issue Oct 10, 2023 · 7 comments

Comments

@josephrony277
Copy link

josephrony277 commented Oct 10, 2023

I'm trying to upload files from different clients to S3 concurrently. Most of the concurrent uploads finish without issues, but for a few uploads, the patch request returns a 408-time-out error after 3 mins. Node js server application has thrown this error below

{
"code": "ECONNRESET",
"name": "Error",
"message": "aborted",
"stack": "Error: aborted\n at connResetException (node:internal/errors:720:14)\n at abortIncoming (node:_http_server:766:17)\n at socketOnClose (node:_http_server:760:3)\n at Socket.emit (node:events:529:35)\n at Socket.emit (node:domain:489:12)\n at TCP. (node:net:350:12)"
}

I tried a fix for the issue #409 and even with the latest version, the issue still remains.

Even when closing the closing the client app during upload, the same error is thrown in the server application.

Fyi,

  • Chunk size configured in the client - 50MB
  • S3 part size configured in the node js app - 50MB
  • Node js app is running as fargate service
@Murderlon
Copy link
Member

Hi, can you try not setting the chunk size on the client, you never want to set this it's always worse unless you are forced to.

I would also considering lowering the S3 part size (for instance 10MB or 20MB), which doesn't have anything to do with the client chunk size and they don't have to be in sync.

Can you see if the issue persists then? Of course, the tus server should work with all settings but I I'm trying to see if it may be a connection error to S3 because of your settings.

@mitjap
Copy link
Collaborator

mitjap commented Oct 26, 2023

@josephrony277 Do you use Amazon S3 or any other provider?

@Murderlon
Copy link
Member

Fairly certain this was fixed in @tus/[email protected] or @tus/[email protected]. We can reopen if someone can reproduce this.

@mitjap
Copy link
Collaborator

mitjap commented Feb 7, 2025

I know this is an old issue but I want to share my thoughts on this if someone comes creeping around :)

Change was made in node v18.0.0 (released on Apr 19, 2022) to enable request timeout by default. This was disabled in previous version. This causes server to close long request and return status code 408. I also received this status code, but most often I just get aborted error with code: 'ECONNRESET'.

Disabling this timeout completely resolves this issue.

const httpServer = tusServer.listen(1080, () => {
    console.log('TUS server listening on port 1080');
});
httpServer.requestTimeout = 0; // disable timeout

Release notes: https://nodejs.org/en/blog/release/v18.0.0
PR: nodejs/node#41263

server.requestTimeout which sets the timeout value in milliseconds for receiving the entire request from the client is now set to 300000 (5 minutes) by default.

If these timeouts expire, the server responds with status 408 without forwarding the request to the request listener and then closes the connection.

@Acconut
Copy link
Member

Acconut commented Feb 10, 2025

Thanks for sharing, @mitjap! Request timeouts are useful to limit a request's duration and avoid leaking resources. Do you know if the request timeout can be set on a per-request basis? Then it could be disabled only for PATCH requests.

@mitjap
Copy link
Collaborator

mitjap commented Feb 11, 2025

I couldn't find a way to configure this on a per-request basis.

@Murderlon
Copy link
Member

Since we already create context with two abort signals and pass it around, we could probably remove the request timeout and do it manually.

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

4 participants