SEZ9 opened a new pull request, #12784: URL: https://github.com/apache/seatunnel/pull/12784
### Purpose of this pull request Closes #12783. @DanielLeens raised these in review on #12631 and #12688. They were non-blocking, and those PRs merged before I sent the fixes, so they are on `dev` now. Four are worth fixing rather than leaving; three smaller ones from the same review rounds ride along because they are in the same files. **Two are documentation contradicting the shipped code.** - Both READMEs said the checkpoint rules use the overview's `updatedAt` as their clock. They use `diagnostics.generatedAt`, and deliberately so: the engine writes `updatedAt` only inside `HazelcastCheckpointOverviewStateStore.updateOverview`, i.e. only when a checkpoint event is recorded, so for a checkpoint hung on its last subtask it stops advancing and would hide the stall the rule exists to find. The docs were describing a design that was tried and rejected during review — @DanielLeens suggested `updatedAt` first, then checked the engine and retracted it. The replacement text records why it is not used, so the inference is not re-made from the field name. - The sample output showed crash-loop wording that no longer exists in the code. I regenerated the sample by running the rule rather than editing it by hand. **Two are rule defects of the same shape: a check running before the one that should bound it.** - `_crash_loop_findings` applied the `restores < CRASH_LOOP_RESTORES` threshold before the exhausted check, so a pipeline at its limit was skipped entirely when the limit was below 2. With `job.retry.times = 1` the budget is spent on the first restore — the reading where the job is certain to die on the next failure — and the output was nothing at all. One restore below the limit stays silent, as before. - A `CANCELING` or `FAILING` pipeline at its limit was told `the next failure ends the job`. It has no next failure. The tearing-down branch now precedes the exhausted one, which is the ordering the end-state check already uses for `FINISHED`/`CANCELED`. **Three smaller ones.** - The job-id pattern ended in `$`, which also matches just before a trailing newline, so `"852\n"` passed validation and was interpolated into the URL path. Now `\Z`. - The `SEATUNNEL_API_BASE` comment listed only `enable-dynamic-port` as a reason the default base can be wrong; a non-empty `seatunnel.engine.http.context-path` is another, since it mounts every servlet under a prefix. - The `generatedAt` plumbing from `/job-info` into the checkpoint rules had no test, only the rule itself did. ### Does this PR introduce _any_ user-facing change? Yes, in `--diagnose` output, in three cases that were wrong before: | job state | before | after | |---|---|---| | pipeline at `restoreCount=1/maxRestoreCount=1` | *no finding at all* | `[error] Pipeline 1 has restarted 1 times, which is its limit: the next failure ends the job.` | | `CANCELING` pipeline at its limit | `[error] ... which is its limit: the next failure ends the job.` | `[info] Pipeline 1 restarted 5 times since it was submitted and is now CANCELING.` | | `--diagnose "852<newline>"` | request sent to `/job-info/852%0A` | rejected before the request, with "is not a job id" | Plus the documentation corrections, which bring both READMEs in line with what the code prints. No change to the engine, connectors or transforms, and no change to any other CLI command. ### How was this patch tested or is it not required? `seatunnel-cli` has no Java component, so the CLI workflow (`.github/workflows/cli.yml`) is the relevant CI: `Test with Python 3.10`, `Test with Python 3.13` and `Lint`. Locally, matching what CI runs: - `python3 -m pytest tests/ -q` — 335 passed - `python3 -m ruff check .` — clean with `ruff==0.16.7`, the version `cli.yml` pins Five new tests. Each of the three behaviour fixes was first reproduced against `dev` and the test asserts the corrected reading, so they fail on the current `dev`: - `test_a_spent_restore_budget_is_reported_even_when_the_limit_is_one` - `test_one_restore_below_the_limit_is_still_silent` — pins that the threshold still applies when the budget is not spent - `test_a_tearing_down_pipeline_at_its_limit_is_not_warned_about_a_next_failure` - `test_a_job_id_with_a_trailing_newline_is_rejected` The two for the `generatedAt` plumbing are coverage rather than a fix, since the plumbing was already correct: - `test_the_engine_clock_is_passed_to_the_checkpoint_rules` - `test_a_checkpoint_stalled_against_the_engine_clock_is_still_reported` — the opposite direction, so the first cannot pass by the rule simply never firing I checked those two are real guards rather than decoration: dropping the `now_ms=` argument makes both fail, and the output shows the symptom of the local clock being used (`has been in progress for 8901h33m`). The engine-side facts cited above are from reading the source; I did not run a cluster for this change. ### 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) — N/A, no new dependency * [x] If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs — done: both `seatunnel-cli` READMEs; this PR is partly a docs correction * [x] If you are contributing the connector code, please check that the following files are updated: — N/A, no connector change 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/connectors) 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) * [ ] Update the [`release-note`](https://github.com/apache/seatunnel/blob/dev/release-note.md). — not possible: `release-note.md` was removed from `dev` in #9304. -- 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]
