SEZ9 opened a new pull request, #12631:
URL: https://github.com/apache/seatunnel/pull/12631
Closes #12630
> **Stacked on #12629.** This needs the `rest.py` added there, so the diff
shown against `dev` includes that PR's commit until it merges. The commit to
review here is the second one. I will rebase once #12629 lands.
## Benchmark first
`benchmark/runner.py --provider bedrock --model
us.anthropic.claude-haiku-4-5-20251001-v1:0 --preset smoke --level l1` (12
tasks, haiku-4-5), on this branch: **pass@1 7/12 (58.3%), 0 misroutes, 0
generation errors.**
**That pass@1 number is not a result and I am not presenting it as one.** I
measured this preset's run-to-run spread on unchanged code and it is about five
tasks: one branch of mine scored 6/12 then 11/12 on consecutive runs with an
identical tree, and `dev` scored 11/12. 58.3% sits inside that band.
More to the point, **this change cannot move the benchmark at all.**
Everything added here is either a new file or a new code path:
`diagnostics/jobs.py` is new, and in `cli.py` the additions are one new
function plus one argparse flag that returns before any provider is
constructed. Nothing on the config-generation path is touched. So the run above
is a no-regression check on code I did not modify, and the honest summary is
"the numbers moved within noise, and there is no mechanism by which they could
have moved."
The real verification is 26 new tests, described below.
## Purpose of this pull request
When a Zeta job is not making progress, the CLI has nothing to say about it.
Finding out why means reading the server log on the master node -- and the two
most common answers are already in the REST API.
`/job-info/<id>` has carried a `diagnostics` block since
#11982/#11983/#11984: per-state entry timestamps at both job and pipeline
level, plus each pipeline's restore counter and limit. `seatunnel --diagnose
<jobId>` reads that and answers by arithmetic:
| | |
|---|---|
| `job_failed` | error code and innermost `Caused by`, via the existing
`parse_error` from #12535 |
| `pipeline_crash_loop` | `restoreCount >= 2` -- the pipeline keeps dying
and being restored, so the job reports RUNNING while making no progress |
| `pipeline_restore_exhausted` | `restoreCount >= maxRestoreCount` --
escalated to error, because the next failure ends the job |
| `stuck_before_running` | too long in
CREATED/PENDING/SCHEDULED/INITIALIZING: the cluster did not grant the slots |
| `stuck_tearing_down` | too long in FAILING/CANCELING/DOING_SAVEPOINT: a
task is ignoring the stop request |
| `no_rows_read` / `rows_read_none_written` | `SourceReceivedCount` /
`SinkWriteCount` |
| `running_duration` | info only -- a streaming job running for hours is
doing its job |
```
$ seatunnel --diagnose 852362670771666945
Job 852362670771666945 mysql-to-doris — RUNNING
[warning] Pipeline 1 has restarted 4 times. It is failing and being
restored
repeatedly, so the job looks alive while making no progress.
(restoreCount=4/10)
```
### Three deliberate properties
**No model.** These conclusions are derived, not inferred -- a restore count
of 7 is a crash loop whatever a model thinks. It also means the flag is handled
before any provider setup and works with no API key and no provider configured,
which is the state an operator is usually in when something breaks
unexpectedly. An LLM explanation over the same evidence is a sensible
follow-up, on top of this rather than instead of it.
**Read-only.** One GET. No submit, no stop, no savepoint. That is what makes
it safe to point at a live production job.
**Non-zero exit only for a genuinely `FAILED` job.** A stuck or
crash-looping job is still a live job, and exiting non-zero for it would break
`seatunnel --diagnose x && ...` for a job that is merely slow.
### Three engine details that are easy to get wrong
I checked each of these against the engine source rather than assuming, and
each is pinned by a test, because all three fail silently:
1. **Row counts are strings.** `BaseService.metricsToJsonObject` calls
`toString()` on every metric leaf, so `metrics["SourceReceivedCount"]` is
`"0"`, not `0`. An int comparison would simply never match and the rule would
never fire.
2. **`Table`-prefixed metrics are not scalars.** `TableSourceReceivedCount`
is an object keyed by table name. Reading it as a count would either raise or
coerce to something meaningless -- a false "no rows read" on a healthy job. The
job-wide scalars are the unprefixed names.
3. **`stateTimestamps` is a state-name map, not a sequence.** The engine
records the last entry into *each* state, so the current state's own key is the
right answer, and the fallback when it is absent must be the newest entry --
not the last key, which is arbitrary and can be days stale.
Also worth noting: `restoreCount` is read with a guard against `bool`, since
`bool` subclasses `int` in Python and an unguarded `int()` would turn a stray
`true` into a restore count of 1.
## Does this PR introduce _any_ user-facing change?
Yes, a new flag: `seatunnel --diagnose <JOB_ID>`. Documented in `README.md`
and `README.zh-CN.md`. No existing behaviour changes.
While documenting it I noticed the README's CLI-arguments block is already
out of date -- it lists a `--sync-catalog` flag that does not exist and omits
`--init` and `--export-metadata`. I left that alone to keep this diff to one
feature; happy to fix it in a separate PR.
## How was this patch tested or verified?
26 new tests, 245 passing in total.
`tests/test_diagnose_job.py` (19) covers the rules as pure functions with an
injected clock, including the three engine details above, the severity
ordering, a single restore *not* being flagged (one restore is ordinary
recovery; flagging it would make healthy clusters look sick), a terminal job
not being measured for stuckness, and a sweep of twelve malformed payloads that
must yield no findings rather than raise -- crashing while diagnosing is
strictly worse than saying nothing.
`tests/test_diagnose_cli.py` (7) drives the flag end to end against a real
loopback `HTTPServer`: URL construction, the 404-versus-other-error split, the
exit codes, an engine that is genuinely not listening, and that a password in a
JDBC URL inside `errorMsg` is redacted before printing -- this output routinely
gets pasted into bug reports. No LLM provider is constructed anywhere in that
file, which is itself part of what is being asserted.
## Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according [New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If you are contributing the connector code, please check that the
following files are updated:
1. Update change log that in connector document. For more details you can
refer to
[connector-v2](https://github.com/apache/seatunnel/tree/dev/docs/en/connector-v2)
2. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
3. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
* [x] Update the
[`release-note`](https://github.com/apache/seatunnel/blob/dev/release-note.md).
--
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]
