SEZ9 commented on issue #12332: URL: https://github.com/apache/seatunnel/issues/12332#issuecomment-5739064412
Thanks @SEPURI-SAI-KRISHNA, the correction is clear and I agree with the narrowed reading: code 17 out of `examineConsumeStats` is about `%RETRY%<group>` only, `setPartitionStartOffset()` runs once per job start rather than on the discovery interval, and a broker-wide route gap fails earlier at `offsetTopics` with `GET_MIN_AND_MAX_OFFSETS_ERROR` instead of rewinding. The remaining window between lines 179 and 180 of the synchronized block on a group with committed offsets is small, but the outcome (empty map read as a cold start) is still wrong, so this stays a valid defect. The revised approach in #12349 also makes sense to me: distinguishing "group never registered, retry topic legitimately absent" from "route lost" by probing the requested topic on the already-open admin client, with no retry and no delay outside the failure branch. The argument that a group cannot have committed offsets without having registered, and registration is what creates the retry topic, is the piece that makes returning the empty map safe in the first case. A few asks to close this out on the issue side: 1. Please update the issue body rather than leaving the correction only in comments. Specifically, replace the `No topic route info in name server for the topic: <topic>` wording with `%RETRY%<group>`, drop the "unlucky once" / discovery-interval argument, and rewrite the "Possible fix" section to the two-branch behaviour you describe here. A short "corrected on 2026-09-17, see comment below" note at the top is enough to keep the history honest. 2. Yes, please move the **What I have and have not verified** caveat to the top of the body. A triager should read "established by construction, not by a captured trace" before the mechanism. 3. If you can reproduce the window in an integration setup (e.g. by killing the name server between the two calls), a captured log excerpt showing the code 17 exception followed by the cold-start read would upgrade this from "by construction" to observed. Not a blocker for the fix, but worth attaching if it is cheap. 4. Confirm that the two tests you named, `testCurrentOffsets_retryTopicMissingWhileRouteHealthyReturnsEmpty` and `testCurrentOffsets_routeOutageSurfacesAsFailure`, are in the current head of #12349 so the issue and PR reference the same regression cover. Once the body is updated I am fine leaving this open and linked to #12349 until that merges. <!-- streview-comment:1171 --> -- 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]
