Vivek1106-04 commented on PR #12418: URL: https://github.com/apache/seatunnel/pull/12418#issuecomment-5829127591
@DanielLeens Thank you for the approval and for checking the source and CI yourself instead of relying on my summary. I have pushed all three of your low-severity items in `550c22568`: 1. **The printed thread count was a lower bound.** Trial setup now resyncs every coordinator, not only the probes, before it prints. Every coordinator has therefore triggered and armed its scheduler by the time the threads are counted, so the line gives the full count. Iteration resyncs still cover only the probes. The thread counts in the PR description's local table were taken before this change, and the description now says they are lower bounds. 2. **The teardown test scanned every thread in the JVM.** The fixture now records the `checkpoint-*` threads that already exist when its setup starts, and counts only threads that appear after that. The teardown test starts a `checkpoint-foreign` thread before the fixture and checks that the test passes while that thread is still alive. A scheduler thread left over from another test class therefore can no longer fail it. 3. **Two constants had the same name.** The benchmark's constant is renamed to `MIN_MEASURABLE_INTERVAL_MILLIS`, and its Javadoc now says how it differs from the fixture's `MIN_CHECKPOINT_INTERVAL_MILLIS`, which is the lowest interval SeaTunnel accepts. The test count in the description is corrected as well. @nzw921rx When you have time, could you look at the current head again? Your concern that the benchmark did not exercise the real engine path shaped the rewrite. The benchmark now runs real `CheckpointCoordinator`s through `tryTriggerPendingCheckpoint` and fakes only the network boundary, and the engine has no production code changes. If anything is still missing, please tell me and I will address it. Thanks to @nzw921rx, @SEZ9 and @DanielLeens for reviewing this so carefully over several rounds. The benchmark is much stronger because of your feedback than the first version I opened. -- 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]
