DanielLeens commented on issue #12332:
URL: https://github.com/apache/seatunnel/issues/12332#issuecomment-5697151641

   The separation belongs inside `currentOffsets`, not at the enumerator 
fallback; the three empty-map cases you identified must remain distinct. One 
important correction after checking current `dev`: 
`RocketMqAdminUtil.topicExist(...)` also catches `MQClientException` with 
`ResponseCode.TOPIC_NOT_EXIST` and returns `false`. It therefore has the same 
route-gap ambiguity and cannot prove that a topic is genuinely absent.
   
   The safe minimal contract is consequently: return an empty map only when 
`examineConsumeStats(...)` completed normally and the requested queues truly 
have no matching committed offsets; any failed lookup, including code 17, must 
surface as `RocketMqConnectorException` rather than be translated into a 
cold-start decision. That preserves the successful empty-offset / newly 
discovered-queue fallback while preventing a failed read from silently 
rewinding a group. If the catch is changed, preserve the interrupt flag before 
wrapping an `InterruptedException`.
   
   Please do not add a retry policy or a public option in this fix. Introduce 
the smallest test seam needed to drive `examineConsumeStats(...)` 
deterministically, then extend the existing enumerator tests to prove: (1) a 
successful filtered empty result still starts at the first offset; (2) a 
code-17 lookup failure does not call the first-offset fallback; and (3) no 
acknowledgement or restore path changes. Because this is an existing connector 
with RocketMQ IT coverage, include a deterministic E2E regression if the 
route-error scenario can be injected without timing dependence. A focused PR 
can then be reviewed on its own data-correctness contract.


-- 
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]

Reply via email to