sarkaramrit1993 opened a new pull request, #29452:
URL: https://github.com/apache/flink/pull/29452
## What is the purpose of the change
When an RPC times out, the message always says "you can try to increase
pekko.ask.timeout". That's only right when the call used the default timeout.
In application mode the calls to the dispatcher (`EmbeddedExecutor`,
`EmbeddedJobClient`, `PackagedProgramApplication`) use `client.timeout`, and
REST handlers use `web.timeout`, so raising `pekko.ask.timeout` does nothing
and sends people the wrong way.
## Brief change log
- `PekkoInvocationHandler` passes the timeout the call actually used into
`resolveTimeoutException`.
- The message now says how long the call waited and names the options that
can apply: `pekko.ask.timeout` by default, `client.timeout` for client
operations or `web.timeout` for REST requests.
I first tried to pick one option by comparing the call timeout with
`pekko.ask.timeout`, but that gives the wrong answer whenever the two are set
to the same value (e.g. both 60 s), so the message names all three instead.
`client.timeout` is a string literal since `flink-rpc-akka` can't depend on
`flink-clients`.
## Verifying this change
This change added tests and can be verified as follows:
- Added
`TimeoutCallStackTest#testTimeoutMessageContainsCallTimeoutAndTimeoutOptions`,
which goes through the real Pekko path with a 42 ms `@RpcTimeout`. It fails on
master and passes with this change.
- `flink-rpc-akka` tests pass.
- Manually verified on a standalone application cluster (WordCount) with
`client.timeout: 1 ms`, with and without `pekko.ask.timeout: 1 ms`. Before:
`DispatcherGateway.submitJob ... timed out. ... try to increase
pekko.ask.timeout.` After: `... timed out after 1 ms. ... increase the timeout
of this call: pekko.ask.timeout by default, client.timeout for client
operations or web.timeout for REST requests.`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
Synchronous RPCs and `callAsync` can still surface a plain timeout without
this message. They go through different code paths, so I left them out of this
PR.
---
##### Was generative AI tooling used to co-author this PR?
- [ ] Yes (please specify the tool below)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
