davidzollo opened a new pull request, #12100:
URL: https://github.com/apache/seatunnel/pull/12100

   ## Purpose
   
   Part of the Zeta-engine-core E2E test initiative, Tier 2: coverage for 
verified-fragile current architecture that has no existing test coverage today. 
Unlike Tier 1 (#12027-#12035, #12095, #12098), which guards past incident 
fixes, this test documents today's actual, verified behavior so an 
unintentional future change to it is caught deliberately instead of silently.
   
   ## Verified finding
   
   `PhysicalPlan#makeJobEndWhenPipelineEnded` is hardcoded `true` with no 
config binding or setter:
   
   ```java
   /** Whether we make the job end when pipeline turn to end state. */
   private boolean makeJobEndWhenPipelineEnded = true;
   ```
   
   `PhysicalPlan#addPipelineEndCallback` uses it unconditionally: the moment 
ANY pipeline in a multi-pipeline job exhausts its restore budget 
(`SubPlan#canRestorePipeline`, gated by `job.retry.times`, default 3) and 
reaches terminal `FAILED`, it calls `updateJobState(JobStatus.FAILING)`. That 
job-level transition's `stateProcess()` then calls 
`jobMaster.neverNeedRestore()` followed by `SubPlan#cancelPipeline` on 
**every** pipeline in the job - including ones that never touched the failure 
and were still healthily `RUNNING` toward their own success.
   
   ## What the test does
   
   Adds `testOneUnrecoverablePipelineForceCancelsHealthySiblingPipeline` to 
`ClusterFaultToleranceTwoPipelineIT`, backed by a new template 
`cluster_batch_one_pipeline_permanently_failed_template.conf`:
   
   - `table_healthy`: an ordinary bounded FakeSource -> LocalFile pipeline, 
paced via `split.read-interval` to stay `RUNNING` for tens of seconds.
   - `table_doomed`: a FakeSource -> Assert pipeline whose Assert sink rule 
(`MAX_LENGTH = 0` against a non-empty generated string) fails deterministically 
on every row, so it permanently fails and exhausts a deliberately small 
`job.retry.times = 2` (3 attempts total) within a few seconds.
   
   Assertions:
   - the job ends `FAILED` overall (not a partial success);
   - the doomed pipeline's own terminal state is `FAILED`;
   - **the crux of the documented behavior**: the healthy pipeline's terminal 
state is `CANCELED`, not `FINISHED` - it never reached its own natural 
completion and was force-cancelled purely as a side effect of its sibling's 
failure;
   - corroborated from the data plane: the healthy pipeline's transactional 
LocalFile sink commits strictly fewer rows than the full expected total, since 
a force-cancel before its own completion (and before any checkpoint interval 
elapses) commits nothing.
   
   Per-pipeline state is read from a `PhysicalPlan`/`SubPlan` reference 
resolved once the job is confirmed `RUNNING` (while the job master is 
definitely still registered) and held for the rest of the test, rather than 
re-resolved through `CoordinatorService#getJobMaster(jobId)` after the job ends 
- the latter races the client observing job completion against the 
coordinator's own executor thread forgetting the completed job 
(`runningJobMasterMap.remove`), both triggered by the same completion event 
with no ordering guarantee between them.
   
   This test intentionally **passes** today by observing the cascade happen 
exactly as coded. If a future change makes pipeline failures isolated (for 
example by making `makeJobEndWhenPipelineEnded` configurable or conditional), 
this test will start failing and must be revisited deliberately.
   
   ## Test plan
   
   - `./mvnw spotless:apply -pl 
seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base -am -nsu` - 
passed, no outstanding formatting diffs.
   - `./mvnw install -pl 
'seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base,!seatunnel-engine/seatunnel-engine-ui'
 -am -nsu -Dmaven.gitcommitid.skip=true -DskipTests -Dspotless.check.skip=true 
-T 3C` - `BUILD SUCCESS`; confirmed the new test method actually compiled (not 
just skipped) by inspecting the resulting `.class` file's bytecode (`javap -p`) 
for `testOneUnrecoverablePipelineForceCancelsHealthySiblingPipeline` and its 
helper `getPhysicalPlan`.
   - Test-only change: no `src/main/**` files touched.
   
   CI will run the new test; no local execution was performed for this PR (per 
this fork's local-verification policy for the Apache SeaTunnel repo, local runs 
are limited to build/format checks and GitHub CI is authoritative for actual 
test execution).


-- 
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]

Reply via email to