[FLINK-40602][runtime] JobManager silently omits running jobs whose JobMaster fails or times out on GET /jobs/overview - #29136
[FLINK-40602][runtime] JobManager silently omits running jobs whose JobMaster fails or times out on GET /jobs/overview#29136sqd wants to merge 3 commits into
Conversation
…obDetails A JobMaster whose requestJobDetails fails or times out is silently omitted from the otherwise successful response, so clients conclude the job is gone. This commit adds a failing test reproducing that.
…ery error Currently a JobMaster whose requestJobDetails fails or times out is dropped from the response while the request still succeeds, so clients of /jobs/overview conclude the job is gone. This commit fails the whole request instead, with an exception naming the job whose details could not be retrieved.
| // silently left out: clients treat absence from this list as the job being gone. | ||
| final List<CompletableFuture<JobDetails>> individualJobDetails = | ||
| new ArrayList<>(jobManagerRunnerRegistry.size()); | ||
| for (JobManagerRunner jobManagerRunner : jobManagerRunnerRegistry.getJobManagerRunners()) { |
There was a problem hiding this comment.
Question: This is the endpoint the Web UI job list polls too, not just the K8s operator. With this change, one running job whose JobMaster is briefly slow or times out on requestJobDetails now takes down the entire overview (500) instead of returning the other healthy jobs. For the operator, failing loudly is clearly the right call; for the UI it's a regression from "one job missing" to "whole page errors." Did you weigh that trade-off?
There was a problem hiding this comment.
Hi, thanks for taking a look! I think "one job missing silently" is dangerous not only for the operator, but also for an engineer who is using the UI to determine what to do. The engineer may draw wrong conclusion and actions from the UI. Failing loud as a 500 for now is an improvement in my opinion.
I agree with you that UX-wise this is a regression. I think something we can do in a followup PR, is to either cache previous good listing in the frontend, and show a banner along the line of "cannot contact jobmanager to update job list, what's currently shown may be stale", or render an empty job list with a similar message.
github-actions
Bot
added
the
community-reviewed
The test accepted any ExecutionException, so an unrelated failure would pass it. This commit asserts the failure names the job whose JobMaster could not be queried.
3 participants
What is the purpose of the change
GET /jobs/overview (backed by Dispatcher.requestMultipleJobDetails) queries every registered JobMaster, but it swallows any failure. A running job whose JobMaster fails or times out on requestJobDetails is therefore silently omitted while the request still succeeds. Clients that use /jobs/overview as the source of truth for job existence (e.g. the Flink Kubernetes Operator) conclude the job is gone.
This pull request makes requestMultipleJobDetails fail the whole request instead, with a FlinkException naming the job whose details could not be retrieved, so an incomplete view is never reported as a successful one.
Brief change log
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
Documentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code Fable 5.1