DanielLeens commented on PR #12896: URL: https://github.com/apache/seatunnel/pull/12896#issuecomment-6289537535
Thanks for the follow-up, @nzw921rx. Here is the reasoning behind keeping the `pipelineNum=1` point, in case it helps the decision (the final call is of course up to the author and the committers). - **What it measures:** with one pipeline the coordinator has a single member to drive, so this point is the "idle member" baseline: the cheapest possible trigger cycle. The 10, 100 and 500 pipeline points show how the cost scales, and the one-pipeline point shows the fixed per-trigger overhead that the scaling points sit on top of. Without it, a regression in the fixed path (timer, trigger bookkeeping) is harder to separate from a regression in the per-pipeline path. - **Cost:** the point is slow only because the interval floor is 200 ms, so 1,000 triggers need about 200 s per trial. That is roughly a third of the added wall time, and it only applies when this class is run on purpose, because `CheckpointSchedulingBenchmark` is not listed in `tools/benchmarks/suites/benchmarks_core.txt`. - **If you prefer to drop it:** the warmup logic itself does not depend on the one-pipeline point (the failure deadline is computed per point), so removing `1` from the `@Param` list would be a small, self-contained change. The trade-off is losing the idle-member baseline and the data point that originally showed the slowest warmup. My earlier review conclusion on head `1914eabf` is unchanged: no blocking issues, and I am fine with either choice here. Since this changes what the benchmark covers rather than correctness, it is a good question for the author and a committer to settle together. -- 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]
