You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/10/03 04:19:40 UTC

[PR] test(ai): stop racing the worker when asserting the stop attribution [rocketmq-dashboard]

unbridled-41 opened a new pull request, #5450:
URL: https://github.com/apache/rocketmq-dashboard/pull/5450

   ## Problem / Evidence
   
   `AiRunServiceTest.stopShouldAbortALiveRunAndLeaveItStoppedWithUserStopTest` fails intermittently in CI, on branches that do not touch this code at all:
   
   ```
   [ERROR] AiRunServiceTest.stopShouldAbortALiveRunAndLeaveItStoppedWithUserStopTest:327
           Expecting Optional to contain a value but it was empty.
   ```
   
   Observed on three PRs of this batch that only change unrelated areas — #5436 (LLM key persistence), #5441 (DLQ resend confinement) and #5447 (alert threshold unit) — so it is a pre-existing flake, not a regression. The assertion under test (pre-fix):
   
   ```java
   RmqAiRun stopped = service.stop(RUN_ID);
   assertThat(registry.handle(RUN_ID))          // line 326
           .get()                               // line 327 — the failure
           .satisfies(handle -> assertThat(handle.abortReason()).contains(AbortReason.USER_STOP));
   ```
   
   ## Root cause
   
   `service.stop(RUN_ID)` → `AgentRunRegistry.stop` → `AgentRunHandle.stop` → `awaitStop()` → `cancelWorker()`, i.e. it interrupts the worker, and the worker's own finalisation is what removes the registration: `AiRunExecutor.execute`'s `finally` calls `registry.finish(run.getId())` (`AiRunExecutor.java:301`), which does `runs.remove(runId)`. The test then looks the run up **again**, so it races with that removal.
   
   Measured on this machine with a temporary probe (removed again): the run reaches `STOPPED`, and the registration is gone **2 ms** later. Locally the assertion usually wins the 2 ms window (the test passed 5/5 and 10/10 here); on a loaded CI runner it loses, which is exactly the recorded failure. Roughly one in ten of this batch's `Backend Tests` runs was red because of it.
   
   ## Fix
   
   Capture the handle **before** stopping and assert the reason on that reference — the same intent, without the race:
   
   ```java
   AgentRunHandle handle = registry.handle(RUN_ID).orElseThrow();
   RmqAiRun stopped = service.stop(RUN_ID);
   assertThat(handle.abortReason()).contains(AbortReason.USER_STOP);
   ```
   
   The user-visible attribution stays asserted through the run row, unchanged: `awaitRunStatus(STOPPED)` and `lastRun().getStopReason() == USER_STOP`.
   
   ## Priority & scoring
   
   PRIORITY 55 = 影响 14 (an unrelated flake turns ~1 in 10 CI runs red, sending contributors after phantom failures and blocking merges) + 波及 10 (every backend CI run) + 可复现 11 (intermittent, mechanism demonstrated above) + 维护价值 20 (a CI signal contributors can trust).
   
   FIX_CONFIDENCE 85: two lines, no behavioural assertion dropped; stability measured with 10 consecutive runs of the class.
   
   ## Tests
   
   ```
   cd server && mvn -o test -Dtest='org.apache.rocketmq.studio.ops.ai.conversation.*Test'
   ```
   
   - Green: `Tests run: 201, Failures: 0, Errors: 0, Skipped: 0` (includes `AgentRunHandleStopTest`, which still pins the handle-level single-stop semantics).
   - Stability: `AiRunServiceTest` run 10 times in a row → `Tests run: 22, Failures: 0, Errors: 0, Skipped: 0` each time.
   - Mechanism proof (temporary probe, not committed): after the run reaches `STOPPED`, `registry.handle(RUN_ID)` is still present immediately and disappears within 2 ms — the window the old assertion raced.
   - `mvn -o checkstyle:check` passes.
   
   ## Risk
   
   None for production code: only the test's assertion target changes. The assertion it makes (the stop is attributed to the user, and the run ends STOPPED/USER_STOP) is preserved.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(ai): stop racing the worker when asserting the stop attribution [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5450:
URL: https://github.com/apache/rocketmq-dashboard/pull/5450#issuecomment-5965546786

   ### CI status of this PR
   
   `Backend Tests (Java 21)` — **pass**; the change-specific command and its result are in the Tests section of the description.
   
   The three remaining red jobs are red for every branch, including `rocketmq-studio` itself, with the same logs:
   
   - `Backend Build (Java 21)` — fails inside the `binary-license-gate` antrun step (`exec returned: 1`); it also fails on a docs-only PR of this batch, and is addressed independently by #5341.
   - `Frontend Build (Node 20)` — fails after a successful build in the license manifest check (`license gate: build artifact verification failed: assets/index-<hash>.js`); it fails identically on a docs-only PR of this batch (run 37088829422) and on trunk, and is addressed independently by #5346.
   - `Frontend Docker Build` — depends on the frontend build above.
   
   This diff touches `server/`, so none of the three can be caused by 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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(ai): stop racing the worker when asserting the stop attribution [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #5450: test(ai): stop racing the worker when asserting the stop attribution
URL: https://github.com/apache/rocketmq-dashboard/pull/5450


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(ai): stop racing the worker when asserting the stop attribution [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #5450:
URL: https://github.com/apache/rocketmq-dashboard/pull/5450#issuecomment-6077327915

   Merged as part of #5440 (squashed into `rocketmq-studio` as 12b63f347).
   
   It stopped `AiRunServiceTest.stopShouldAbortALiveRunAndLeaveItStoppedWithUserStopTest` from racing the worker, which was making that case flaky.
   
   We are consolidating same-author changes that touch the same files into one commit rather than landing several near-identical ones on the trunk; your commit is preserved as the author of the corresponding part of that change. Closing - no action needed, and thanks for the fix.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org