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.

Sign up for GitHub

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

Jump to bottom

Set proxy read timeout to 1 hour #2336

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Set proxy read timeout to 1 hour #2336

merged 2 commits into from
Jan 20, 2022

Conversation

vicky-g
Copy link
Contributor

@vicky-g vicky-g commented Jan 19, 2022 •
edited

Description

This PR is to bump the read timeout to 1 hour (3600 seconds). This is the timeout set at the load balancer level. The default timeout is 60s. (reference)

Tests

Set the proxy read timeout to 4s:

proxy_read_timeout 4;

Created a route that waits n ms:

  app.get(
    '/test',
    handleResponse(async (req, res) => {
      await new Promise((resolve) => setTimeout(resolve, req.query.duration))

      return successResponse({
        msg: `you should see this after ${req.query.duration}ms`
      })
    })
  )

Hit route at duration=100 (under read timeout):

{
    "data": {
        "msg": "you should see this after 100ms"
    },
    "signer": "0x11e486b1532e3f5a0090266e550a7e15d9721acf",
    "timestamp": "2022-01-20T18:56:33.759Z",
    "signature": "0xd789898c9fa1b14541674cd64578771002ad7a29f9fdd9c62e35bd6c2d4b5a084d540482b626fc4eaf8efa13f6f7332cdc094623436d4185d1c91bcf026796d51c"
}

Hit route at duration=4010 (over read timeout):
Proxy response:

<html>

<head>
	<title>504 Gateway Time-out</title>
</head>

<body>
	<center>
		<h1>504 Gateway Time-out</h1>
	</center>
	<hr>
	<center>openresty/1.19.9.1</center>
</body>

</html>

Proxy logs:

2022/01/20 18:53:22 [error] 10#10: *1652 upstream timed out (110: Operation timed out) while reading response header from upstream, client: 73.222.231.193, server: , request: "GET /test?duration=4010 HTTP/1.1", upstream: "http://127.0.0.1:3000/test?duration=4010", host: "34.133.121.197:4000"

How will this change be monitored?

Will monitor CN5 stage node to see if any unexpected errors come about via the error.log file and sentry alerts

@vicky-g vicky-g changed the title set proxy read timeout to 1 hour Set proxy read timeout to 1 hour Jan 19, 2022
@vicky-g vicky-g added the content-node Content Node (previously known as Creator Node) label Jan 20, 2022
@vicky-g vicky-g marked this pull request as ready for review January 20, 2022 18:59
dmanjunath
dmanjunath approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice testing! also gonna tag @cheran-senthil to make sure discovery does this because we've seen this exact error on discovery

@vicky-g vicky-g merged commit 70f1258 into master Jan 20, 2022
@vicky-g vicky-g deleted the vg-fix-read-sync-timeout branch January 20, 2022 19:08
@cheran-senthil
Copy link
Contributor

Nice, I'll make this adjustment, thanks for tagging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmanjunath dmanjunath dmanjunath approved these changes

@cheran-senthil cheran-senthil Awaiting requested review from cheran-senthil

Assignees
No one assigned
Labels
content-node Content Node (previously known as Creator Node) size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@vicky-g @cheran-senthil @dmanjunath