You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Wang1rrr (via GitHub)" <gi...@apache.org> on 2026/09/22 04:05:22 UTC

[PR] refactor(message): drop the unreachable message-track fallback [rocketmq-dashboard]

Wang1rrr opened a new pull request, #4854:
URL: https://github.com/apache/rocketmq-dashboard/pull/4854

   <!-- Make sure the base branch is `master`: that is the RocketMQ Studio trunk. -->
   
   ### Which Issue(s) This PR Fixes
   
   - Fixes #4851
   
   ### Brief Description
   
   `RocketMQMessageProvider` has carried a private, 41-line fallback for building a message's consumer status from the broker since #798, and no request has ever reached it:
   
   ```java
   private List<ConsumerStatusVO> fallbackConsumerStatus(DefaultMQAdminExt adminExt, MessageExt message) { ... }
   private DeliveryStatus mapTrackType(TrackType trackType) { ... }
   ```
   
   `fallbackConsumerStatus` has no caller in `server/src` (main or test) and `mapTrackType` is called only from inside it. It was already caller-less in the commit that introduced it (`dc49e1a2`) and survived `ceef946f` (#1231) unchanged. The live path builds consumer status from parsed trace-topic records instead (`buildConsumerStatus`, `:777`).
   
   The reason this is worth removing rather than leaving alone is that the dead mapping is wrong, and it reads exactly like implemented behaviour:
   
   | `TrackType` | mapped to | means |
   | --- | --- | --- |
   | `CONSUMED`, `CONSUME_BROADCASTING`, `CONSUMED_BUT_FILTERED` | `success` | delivered |
   | `NOT_CONSUME_YET`, `PULL`, `NOT_ONLINE` | `pending` | not delivered yet / no consumer online |
   | `null` | `pending` | could not tell |
   | **`UNKNOWN`** (via `default`) | **`failed`** | **could not tell** |
   
   `TrackType` has exactly seven members in rocketmq-tools 5.5.0, so `UNKNOWN` is the only value that reaches `default` - and `DefaultMQAdminExtImpl.messageTrackDetail` sets `UNKNOWN` as the *initial* value on every group's `MessageTrack` before it queries anything, replacing it with `NOT_ONLINE` only when the broker answers 206 (`CONSUMER_NOT_ONLINE`). Any other failure - unreachable broker, timeout, `RemotingException` - leaves it at `UNKNOWN`.
   
   So the same "we could not determine this" state is `pending` when spelled `null` and `failed` when spelled `UNKNOWN`. Wired in as written, that would put a red 失败 (`DELIVERY_STATUS_MAP`, `web/src/pages/instance/message.tsx:108`) on **every** consumer group of the topic, on exactly the clusters where the admin RPCs are failing.
   
   **The change** deletes both methods and the two imports that become unused (`org.apache.rocketmq.tools.admin.api.MessageTrack` and `TrackType`). No behaviour changes, because nothing could call them.
   
   If the community would rather have the fallback than not, that is a separate and deliberate change, and the issue lists what it needs: `UNKNOWN` mapped to `pending` (matching the `null` case here and the `pending` fallback `AliyunConverters:327` already uses), a decision on the cost - `messageTrackDetail` issues one `queryTopicConsumeByWho` plus one `examineConsumerConnectionInfo` **per subscribed group**, from the `TOPIC_NOT_EXIST` / `NO_MESSAGE` branch at `:600`, i.e. the slow path of a page already blocked on RPCs - and a way to get the `MessageExt`, which `resolveMessageStoreTimestamp` (`:670`) already fetches and discards. I am happy to send that instead if it is wanted; this PR just stops shipping the trap.
   
   ### How Did You Test This Change?
   
   ```
   cd server
   mvn -o -B test "-Dtest=RocketMQMessageProvider*,Message*Test,*TraceTest"
    Tests run: 137, Failures: 0, Errors: 0, Skipped: 0
    BUILD SUCCESS
   ```
   
   That covers `RocketMQMessageProviderTest` (56), `MessageServiceTest`, `MessageControllerTest` and the seven message/trace AI tool handler suites. Checkstyle runs in the `validate` phase (`failsOnError=true`, `includeTestSourceDirectory=true`), so the build also proves no unused import or reference survived.
   
   Reachability was established before deleting, not assumed:
   
   ```
   rg -n "fallbackConsumerStatus|mapTrackType|messageTrackDetail" server/src
   # three hits, all inside the two methods themselves
   ```
   
   and the removal script asserts that none of `MessageTrack`, `TrackType`, `fallbackConsumerStatus`, `mapTrackType` or `messageTrackDetail` appears anywhere in the file afterwards, so a half-deletion cannot slip through. Checkstyle's `UnusedImports` is the second net: leaving the imports behind fails `validate` (confirmed while preparing this change).
   
   No new tests: there is no reachable behaviour left to pin. The trace parsing that *is* live stays covered by the existing `RocketMQMessageProviderTest` cases, all of which pass unchanged.
   
   ### Checklist
   
   - [x] One coherent change; unrelated modifications are not bundled in
   - [x] Commit subject follows Conventional Commits (`feat:` / `fix:` / `refactor:` / `chore:` / `docs:` / `perf:`)
   - [x] Tests added or updated for non-trivial changes, test methods named `...Test` - none added; this deletes unreachable private code and the 137 existing message/trace tests pass unchanged
   - [x] New UI text has both Chinese and English entries under `web/src/i18n/` (no UI change)
   - [x] Architecture constraints stay green (`mvn test` runs the ArchUnit checks)
   - [x] New source files carry the ASF license header (no new source files)
   - [x] Documentation touched where behaviour changed (README / `docs/` / in-app help) - none needed; no reachable behaviour changed, so no documented contract did either
   


-- 
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] refactor(message): drop the unreachable message-track fallback [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #4854:
URL: https://github.com/apache/rocketmq-dashboard/pull/4854


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