nzw921rx commented on PR #12418: URL: https://github.com/apache/seatunnel/pull/12418#issuecomment-5776752201
> @nzw921rx @DanielLeens On the question of which direction this harness should take: I would take the queuing-faithful standalone harness, and I can push it here. > > Taking the two options as Daniel framed them. > > **Driving the real `CheckpointCoordinator#scheduleTriggerPendingCheckpoint`.** The hook is there and is already `@VisibleForTesting protected`, so reaching it is not the problem. The problem is what it pulls in: `tryTriggerPendingCheckpoint` on the happy path calls `createPendingCheckpoint` and `startTriggerPendingCheckpoint`, so the sample would include checkpoint-id allocation, a real `PendingCheckpoint`, checkpoint storage writes and `CheckpointBarrierTriggerOperation` RPCs to task locations, on top of a mocked `CheckpointManager`, execution plan and task locations of the kind `CheckpointCoordinatorTest` builds with Mockito. > > Two reasons I do not think that answers the question this benchmark was built for. First, the measurement stops being scheduling delay: barrier plus storage dominates, and the numbers would move with storage and RPC behaviour rather than with the scheduling model. Second, we already have that measurement - `CheckpointingTimeBenchmark` is barrier-ACK dominated, and its `CheckpointBenchmarkTrigger` reflects straight into `createPendingCheckpoint` and `startTriggerPendingCheckpoint`. Routing this benchmark through the same machinery would mostly reproduce it at P=1. > > **The queuing-faithful harness.** @nzw921rx's objection is that the current state arms one trigger-like task per pool and so measures thread wake-up latency rather than queueing, and that is correct as written. Daniel's Issue 1 proposal fixes it directly: arm both a trigger-like task and a watchdog-like task per pool, with a duty cycle long enough that both of a pool's two workers can be occupied, so the measured task actually has to queue behind them. That is faithful to the engine's real pattern - the trigger re-arms itself at `scheduleTriggerPendingCheckpoint(checkpointType, 500L)` while a checkpoint is pending, and the timeout watchdog is armed per checkpoint, so two timers against a two-thread pool is what a busy pipeline actually looks like - and it keeps the measured quantity as scheduling delay. > > It also fixes the reading of the current sweep, which is Daniel's other point: the knee at P=500 in what I posted is thread and CPU contention from roughly 1000 live threads in one JVM, not queue pressure, and the harness as it stands cannot separate those. With both timers armed it can. > > So unless you want the full-fidelity version for its own sake, I will: > > 1. add the watchdog-like task alongside the trigger-like one, with a configurable duty > cycle, so a measured task can genuinely queue behind both workers of a pool; > 2. re-run the sweep and replace the numbers in the description, including the numbers I > posted for [[Improve][Zeta] Share checkpoint trigger scheduling across active pipelines #12165](https://github.com/apache/seatunnel/pull/12165), since those were taken on the current harness and the P=500 knee > in them should be read as thread contention rather than queueing; > 3. say in the class Javadoc what the harness does and does not model, so the next reader > does not have to rediscover it. > > If you would rather have the real-engine-path version, or both arms, say so and I will do that instead - I would just rather not put a number in front of reviewers without being able to say which of the two it measures. > > Separately, this PR's earlier three items remain addressed at this head: 4g heap, README reverted, and the class registered in the `benchmarks.yml` choice list. The `benchmarks_core.txt` decision is still yours; my view is to leave it out until the matrix is settled, since eight parameter configurations at roughly four minutes each is about 32 minutes per Java version against the 240-minute nightly budget. Thank you, please take a look at my design above. -- 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]
