SEPURI-SAI-KRISHNA opened a new pull request, #12392: URL: https://github.com/apache/seatunnel/pull/12392
### Purpose of this pull request This is the follow-up I committed to on #12323, which fixes the two remaining gaps I found while sweeping that file and deliberately kept out of that PR to keep its scope tight. Both were listed there as points 2 and 3, and @DanielLeens recorded them as non-blocking in his review. **`currentOffsets` was not retry-wrapped while its neighbour was.** In `getRocketMqConsumerData`, the `offsetTopics` lookup is wrapped in `RetryUtils.retryWithException`, and the `currentOffsets` call immediately below it was not, although both read broker metadata that can be briefly unavailable. They now use the same `RetryMaterial`. This matters more since #12349. There, `RocketMqAdminUtil.currentOffsets` stops reporting a failed lookup as an empty map and surfaces it as `RocketMqConnectorException` instead. That is the right behaviour for the production caller, but it means an unwrapped call here fails the test where the wrapped neighbour one line up would have retried. The `RetryMaterial` already in place retries exactly `exception instanceof RocketMqConnectorException`, so this is the consistent spelling rather than a new policy. **`checkOffsetNoDiff` has the tightest window in the file.** It reads `offsetTopics` and then `currentOffsets` under `Awaitility` with a 30 second ceiling. Every other window in this file is 60 seconds or longer, up to 5 minutes, and while working on #12322 I measured a name server route gap that lasted 4 minutes 24 seconds. A gap of that size fails this check. The fix confirms the route before entering the window rather than widening it, using the `waitForTopicRoute` helper already in the file. That follows the approach #12323 established, and keeps the assertion window itself tight so a genuine offset mismatch still fails fast. I want to be precise about the evidence for each, as I was on #12323. The `checkOffsetNoDiff` window is latent rather than demonstrated: `testSourceRocketMqTextToConsoleWithOffsetCheck` did not fail once across the 12 daily `Schedule Backend` runs I sampled for #12322. That is why it was not folded into #12323. The `currentOffsets` call has been seen failing tests, but I do not want to claim it as evidence for a retry, because it is not. On the first commit of #12349, before that PR was corrected, `testSinkRocketMq` failed 7 of 7 at `getRocketMqConsumerData` with `ROCKETMQ-09`. The cause there was the never-created `%RETRY%` topic, which is a standing condition rather than a transient one, so a retry would only have delayed the same failure. #12349 fixes that cause directly. What this change covers is the genuinely transient case, and that has not been separately demonstrated on this call. It is a consistency fix with a plausible benefit, not a fix for an observed failure. ### Does this PR introduce _any_ user-facing change? No. Test-only, in a single E2E class. No production code, config option, or documented behaviour is touched. ### How was this patch tested? `./mvnw -q -DskipTests verify -pl seatunnel-e2e/seatunnel-connector-v2-e2e/connector-rocketmq-e2e` on JDK 11 passes, which covers the enforcer checks, `spotless:check` and compilation. `spotless:check` was also run separately with up-to-date caching disabled. No new test is added, and this is the case the test-class convention is about: the change is to `RocketMqIT` itself, so the IT is both the thing being changed and the coverage. The `rocketmq-connector-it` legs on this PR's own CI exercise both paths: `getRocketMqConsumerData` is reached from `testSinkRocketMq`, `testTextFormatSinkRocketMq` and `testSinkRocketMqMessageTag`, and `checkOffsetNoDiff` from `testSourceRocketMqTextToConsoleWithOffsetCheck`. -- 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]
