Open jrhee17 opened 1 month ago
grpc-timeout
is not just a number. A suffix representing a time unit such as s
, m
or n
is appended to the value. It would be a hassle to parse the value.
How about passing the parsed grpc-timeout
value to the interface?
interface GrpcClientTimeoutHandler {
static GrpcClientTimeoutHandler enabled() {
return Function.identity();
}
static GrpcClientTimeoutHandler disabled() {
return (ctx, timeout) -> -1;
}
static GrpcClientTimeoutHandler withBuffer(long bufferMillis) {
return (ctx, timeout) -> timeout + buffer;
}
long apply(ServiceRequestContext ctx, long timeoutMillis);
}
How about passing the parsed grpc-timeout value to the interface?
The original intention was that users may want to know other header values when clamping the timeout.
However, I realized that this information is in ctx.request().headers()
anyways, so the interface you suggested seems better. (The only concern I have is nanos
vs. millis
)
Also possibly related https://github.com/line/armeria/issues/3155
er. (The only concern I have is nanos vs. millis
If it is considerable, Duration may be an alternative.
I wonder if any of the server side flags affect the timeout behavior?
etc. IIRC some of those caused connection to be terminated pre-maturely before client initiates a timeout.
Timeout does not cause connections to be terminated. In HTTP/2, the timeout sends an error response and the connection will be reused.
I wonder if any of the server side flags affect the timeout behavior?
Possible. We are going to provide an option to allow grpc-timeout
but the maximum value is limited by defaultRequestTimeoutMillis
.
Currently, armeria has a useClientTimeoutHeader option to decide whether to respect the
grpc-timeout
request header or not.Since
useClientTimeoutHeader = true
delegates the timeout value to the client-side, it is potentially dangerous. On the other hand, settinguseClientTimeoutHeader = false
completely ignores the client header and Armeria'srequestTimeout
is used.We may want to offer users a middle ground: Armeria server tries to respect the
grpc-timeout
header, but clamps the timeout according to a bound.In order to support this, we may deprecate the previous
useClientTimeoutHeader
and accept a function which determines how to decide the request timeout. This might also be useful if users want to set timeouts depending on the remote.ref: https://discord.com/channels/1087271586832318494/1245876539682324642/1245948161835667528