feat(proxy) implement upstream timeouts #2036
Conversation
thibaultcha
force-pushed
the
feat/upstream-timeouts
branch
from
4b562d2 to
a6f0cb3
Compare
kong/dao/migrations/cassandra.lua
Outdated
| upstream_connect_timeout = 60000, | ||
| upstream_send_timeout = 60000, | ||
| upstream_read_timeout = 60000, | ||
| }, { id = row.id }) |
There was a problem hiding this comment.
minor: no need to hardcode those values, see https://github.com/Mashape/kong/blob/a6f0cb38661a09cd91802fbdd4ba8c1c696b17d0/kong/dao/migrations/cassandra.lua#L163-L179
kong/dao/migrations/postgres.lua
Outdated
| upstream_connect_timeout = 60000, | ||
| upstream_send_timeout = 60000, | ||
| upstream_read_timeout = 60000, | ||
| }, { id = row.id }) |
kong/dao/schemas/apis.lua
Outdated
|
|
||
| local function check_u_int(t) | ||
| if t < 1 or t > 2^31 - 1 or math.floor(t) ~= t then | ||
| return false, "must be an integer between 1 and 2^31 - 1" |
There was a problem hiding this comment.
error message to technical. And in general (matter of taste) I prefer this; https://github.com/Mashape/kong/blob/a6f0cb38661a09cd91802fbdd4ba8c1c696b17d0/kong/dao/schemas/upstreams.lua#L4-L6
|
|
||
| ok, err = set_timeouts(addr.connect_timeout / 1000, | ||
| addr.send_timeout / 1000, | ||
| addr.read_timeout / 1000) |
There was a problem hiding this comment.
Can't we do these calculations at config-load time, instead of on each request? Should be an easy fix with the new caching (the way I intend to do it)
There was a problem hiding this comment.
I mean the refactor of the cache scheduled for 0.11
There was a problem hiding this comment.
Can't we do these calculations at config-load time, instead of on each request?
I'd rather not. It'd be harder to track in case something goes wrong or if we start loading APIs from another place and forget to do the conversion, etc...
Should be an easy fix with the new caching (the way I intend to do it)
I think we should sync up on that, because the way I understood it, I was supposed to work on the new caching in 0.11? BTW, I cannot know which way you intend to do it if you do not elaborate.
Tieske
added
pr/changes requested
thibaultcha
force-pushed
the
feat/upstream-timeouts
branch
from
a6f0cb3 to
7327fbf
Compare
thibaultcha
removed
the
pr/changes requested
* add new fields to APIs for connect/send/read timeouts * fields are in ms precision, and must be greater than 0 * update CHANGELOG.md with new feature
thibaultcha
force-pushed
the
feat/upstream-timeouts
branch
from
7327fbf to
f8408c7
Compare
|
@Tieske Thanks for the review! |
Summary
This implements timeout options between Kong and the upstream services/APIs. It adds 3 new fields to the API definition:
upstream_connect_timeout,upstream_send_timeout, andupstream_read_timeout.Those fields use a ms precision, and this take an integer value between 1 and 2^31 - 1.
When not specified, the default value is
60000(60 seconds) for all 3 fields.Full changelog