SEZ9 commented on PR #12418: URL: https://github.com/apache/seatunnel/pull/12418#issuecomment-5903487021
Thanks @Vivek1106-04 — `550c22568` closes out the three low-severity items from my last pass: 1. **Thread count no longer a lower bound.** Resyncing every coordinator, not just the probes, in trial setup before printing means the count reflects all armed schedulers at `pipelineNum`. Keeping the iteration resync probe-only is the right call since it stays outside the timed region. Marking the PR description's local table as lower bounds is the honest fix for numbers taken before this change. 2. **Teardown test scoped to the fixture's own threads.** Snapshotting pre-existing `checkpoint-*` threads and asserting against a live `checkpoint-foreign` thread makes this a real regression test rather than a rename; a leftover scheduler from another test class can't fail it anymore. 3. **Constant naming.** `MIN_MEASURABLE_INTERVAL_MILLIS` with a Javadoc contrasting it against the fixture's `MIN_CHECKPOINT_INTERVAL_MILLIS` removes the ambiguity. Nothing further from me on these. I see @nzw921rx has left a couple of new comments on the current head. I'll leave those between the two of you — please respond on the thread (or push a follow-up if you agree) so we have a clear final state before this is merged. <!-- streview-comment:1428 --> -- 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]
