SEPURI-SAI-KRISHNA commented on issue #12332:
URL: https://github.com/apache/seatunnel/issues/12332#issuecomment-5741198143

   All four done or answered.
   
   **1 and 2.** The body is updated. The verified/not-verified caveat is now 
the first thing under "What happened", ahead of the mechanism, with a dated 
correction notice linking to the correction comment so the history stays 
readable. The `<topic>` wording is replaced by `%RETRY%<group>` throughout, 
including in the Error Exception block. The "unlucky once" and 
discovery-interval argument is gone, replaced by the narrow window it actually 
is: between `run():179` and `run():180` of one synchronized block, on a group 
that already has committed offsets. The "Possible fix" section now describes 
the two-branch behaviour and points at #12349.
   
   **4. Confirmed.** Both tests are in #12349's current head, `f98275e5`, in 
`RocketMqAdminUtilTest`:
   
   * `testCurrentOffsets_retryTopicMissingWhileRouteHealthyReturnsEmpty`
   * `testCurrentOffsets_routeOutageSurfacesAsFailure`
   
   alongside 
`testCurrentOffsets_otherResponseCodeSurfacesEvenWhenRouteHealthy`, which pins 
that the route probe applies only to code 17, and the two pre-existing 
successful-lookup cases. I also checked they fail for the right reasons rather 
than passing by luck: removing the route probe fails only the outage test, and 
removing the code 17 branch entirely fails only the retry-topic test.
   
   **3. I do not think I can do this cheaply, and I would rather say so than 
let it sit as an open promise.** The window is between two statements in the 
same synchronized block, so reproducing it means killing the name server inside 
that gap. `RocketMqIT` shares one container across every engine leg, so 
stopping the name server mid-test would take the rest of the class down with 
it, and a delete-and-restore of just `%RETRY%<group>` races the broker's 
re-registration heartbeat. I could force it with a test seam that injects the 
failure, but that proves the handler's behaviour rather than the window's 
existence, and the unit tests already do that.
   
   So the "by construction" caveat stays, and it is now the first thing a 
triager reads. If you would still like the injected version, say so and I will 
add it, with the caveat written on it.
   


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