SEZ9 commented on PR #12438:
URL: https://github.com/apache/seatunnel/pull/12438#issuecomment-5882382008
@SEPURI-SAI-KRISHNA thanks for re-checking the reachability argument against
`topicRouteAvailable` and the `groupOffsets.isEmpty()` fallback rather than
just taking the wording. The paragraph you quoted for Issue 1 reads correctly
to me: it states the transition is reachable on a multi-broker layout, explains
why the guard does not cover it, and names the consequence (empty map read as a
cold start, full rewind) and the fix direction (a `continue` that keeps the
earlier offsets). I will confirm that text against the `RocketMqAdminUtil.java`
comment at the current head (533cfde8) before marking PR12438-F1 resolved, but
I do not expect any further changes.
On the `continue` change plus the two-topic `RocketMqAdminUtilTest` case:
keep it out of this PR. Your reasoning is right that this PR is comment-only
and that change alters production behaviour on a replay-hazard path, so a
separate issue and PR after this lands is the right shape. No need to close and
refile a combined one.
On Issue 2 (PR12438-F2): your comment appears to have been cut off partway
through "the same pattern that we", so I only see that you agreed with the
stronger version. No need to paste the wording here; I will check the
`RocketMqIT` comment in the diff at the current head and confirm it no longer
references the PR number or narrates what the code used to be, and that it
describes an empty map as a full rewind of every topic, matching the `if
(groupOffsets.isEmpty()) { listOffsets(queues, CONSUME_FROM_FIRST_OFFSET) }`
behaviour you cited. If the truncated part of your comment contained anything
beyond that, feel free to add it.
Assuming the diff matches, I expect this is good to go.
<!-- streview-comment:1386 -->
--
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]
