Vivek1106-04 commented on PR #12418: URL: https://github.com/apache/seatunnel/pull/12418#issuecomment-5789833424
@nzw921rx @DanielLeens Before choosing between the two fixtures I built both and ran them, on `dev` and on #12165, because a few of the open questions could only be settled by running code. Short version: neither fixture works exactly as proposed, but a variant does, it needs no production change, and the same source runs on both revisions. ## 1. Measure the real periodic trigger instead of injecting one Injecting a trigger per sample fights the interval guard in `tryTriggerPendingCheckpoint`. With a long interval the injected trigger is re-armed instead of run; with the 10 ms interval the coordinator re-arms itself every 10 ms and races the next sample, which is what `cancelScheduledCheckpointTriggers()` was for. That hook needs a new field on `dev` (the trigger's `ScheduledFuture` is discarded at `CheckpointCoordinator.java:550`) and a different implementation on #12165, so it would be production code on both sides. The alternative needs neither. Let the coordinator run its own periodic trigger at its configured interval, and observe it: a sample is the time `pendingCounter` goes 0 -> 1, minus the previous trigger time plus the interval. A sample is kept only if the previous checkpoint completed before the next one was due, so every kept sample went through the normal path, including the lock and the executor submission, rather than the 500 ms pending re-arm. The field is read, never written. Nothing in `CheckpointCoordinator` changes. ## 2. The no-cluster fixture cannot sustain samples `pendingCounter` is incremented synchronously on the trigger path and only comes back down when the checkpoint completes, or through `cleanPendingCheckpoint`, which sets `shutdown = true`. With no tasks, the checkpoint never completes, and from the second sample on every trigger takes the `pendingCounter > 0` re-arm and never reaches the lock or the executor. So each sample's checkpoint has to really complete. Faking the tasks entirely is also blocked: the fake network has to return an `InvocationFuture`, and in Hazelcast 5.1 that class is `final` with a package-private constructor that dereferences a live `Invocation`. A real member is needed to produce one. ## 3. The mini-cluster does not scale to the sweep With `SeaTunnelCheckpointEnvironmentContext` and real submitted jobs (`-XX:ActiveProcessorCount=4`, 4g, as in the benchmark): - P = 1 works: the real trigger is observed and checkpoints take about 11 ms. - P = 100 does not: 361 s from submission to all jobs RUNNING, then zero completed checkpoints in a 60 s window, with `TaskAcknowledgeOperation got rejected before execution due to not starting within the operation-call-timeout`. P = 500 is out of reach on the benchmark's own CPU budget. ## 4. What works: a real member, real coordinators, fake tasks One real SeaTunnel member provides the real checkpoint storage, IMap, monitor service and engine context, so on #12165 the coordinators use the real member-wide scheduler. A real `CheckpointManager` builds real coordinators through the existing constructor. The only fake is an anonymous subclass overriding the existing `protected sendOperationToMemberNode`: it answers each barrier through the public `acknowledgeTask`, and returns a real completed future from a local no-op operation. No Mockito, no new dependency, no change to engine code. P = 500 as 500 jobs of one pipeline, 10 s interval, starts staggered across one interval, 60 s window: | | `dev` | #12165 | |---|---|---| | checkpoint scheduler threads | 1000 | 10 | | live threads, whole JVM | 1137 | 152 | | kept samples | 499 of 499 | 500 of 500 | | average checkpoint | 6.3 ms | 5.6 ms | | live heap after full GC | 42 MB | 41 MB | ## 5. Two things the runs forced on the design **Pipelines must be spread across jobs.** The same 500 pipelines as one job averaged 12 s per checkpoint, and no sample was usable. As 500 jobs it was 5.6 ms. I have not proven the cause; my suspicion is that every completion updates one per-job entry in the checkpoint overview IMap. Either way the benchmark should model many jobs, which is also what "500 active pipelines" in #12122 means in practice. The interval is set from the same constraint: at P = 500, a 1 s interval is 500 real checkpoint completions per second and storage cannot keep up, so the sweep uses a fixed 10 s. **Absolute delay numbers must come from CI.** On macOS every timed wait fires about 4-5 ms late, and a plain `ScheduledThreadPoolExecutor` shows the same, so locally the delay distribution sits on the OS timer floor and hides a microsecond-scale difference. The thread counts and the checkpoint completion figures above are valid locally; the delay comparison will come from the Linux runners through the `benchmarks.yml` entry. ## Costs, stated up front - Two internal fields are read by reflection: `pendingCounter` on the coordinator, and `barrier` on `CheckpointBarrierTriggerOperation`, which has no getter. Read-only, but a rename breaks the benchmark, and I would add a check that fails loudly if either field is missing. - Fake tasks acknowledge immediately, so the numbers are a best case for the completion side. The scheduling side is fully real. - The fake encodes two pieces of the task protocol, barrier acknowledgement and `READY_START` reporting, which would need updating if those change. ## Next Unless you see a problem with this, I will replace the standalone harness in this PR with this fixture: sweep P over 1 / 10 / 100 / 500 as jobs of one pipeline, report scheduler thread count and the scheduling delay distribution, fail the run if kept samples drop below a threshold rather than report a number from the re-arm path, and keep the benchmark in `src/main` with no new dependencies. `checkpointSingleInput` stays untouched. -- 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]
