[PR] [ISSUE #6835] report a failed NameServer registry probe instead of an… [rocketmq-dashboard]

["Kris20030907 (via GitHub)" <gi...@apache.org>]

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

   issue #6835
   ## Solution
   
   A probe that cannot complete degrades to a single unavailable placeholder row
   carrying what the registry itself knows (`RegistryProbeRunner.unavailableEntry`):
   
   - `name` / `endpoint` from the registry entry
   - `status: warning` with empty `brokers` / `proxies` / `nameServers` / `tpsHistory`
   - `id`, `nsClusterName`, `type`, `version` left null — no live cluster was
     discovered; `id == null` marks the row as not an actionable cluster
   
   An empty result now always means "the probe succeeded and found no clusters".
   All three failure paths (queue rejection, deadline, probe exception) funnel into
   the same builder, and `probeRegistryEntry` no longer catches `Exception`, so
   degradation has a single site instead of three.
   
   Web (small but required to keep the UI honest):
   
   - the NameServer tab status column shows the probe result, so a placeholder
     renders as "Warning" instead of the stale status stored at registration time
   - "Config Diff" is enabled only when a cluster id was actually discovered — a
     placeholder has no cluster to diff against
   
   The page header counters ("Total 0 ...") are computed from the instance-path
   cluster list and are unaffected — pre-existing behavior.
   
   ## Tests
   
   - `RegistryProbeRunnerTest`: a failing probe, a timed-out probe and a saturated
     probe queue each keep an unavailable row instead of dropping the entry
   - `ClusterServiceRegistryTest`: probe failures degrade to a row instead of being
     skipped; blocked probes still surface per entry while the pool stays bounded
   - `ClusterPage.test.tsx`: an unprobed entry shows "Warning" and keeps "Config Diff"
     disabled
   
   Full server suite: 3738 tests pass. Cluster/clients web suites: 82 tests pass.
   
   ## Verification
   
   Against a local cluster with two registered NameServers (`e2e-ns` alive,
   `dead-ns` on a closed port):
   
   - **dead path** (connection refused): `GET /api/clusters/registry` now returns
     both entries — `dead-ns` as `{"name":"dead-ns","endpoint":"127.0.0.1:9877","status":"warning","id":null,"brokers":[]}`
     (before: the row was absent entirely)
   - **hung path** (TCP connect succeeds, no response): the same placeholder row is
     produced via the deadline, server log shows
     `NameServer registry probe failed for dead-ns ... wait response on the channel <127.0.0.1:9877> timeout`
   
   Before/after screenshots of the NameServer tab attached: before — both entries
   green "Running"; after — `dead-ns` shows "Warning" with "Config Diff" disabled.


-- 
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] [ISSUE #6835] report a failed NameServer registry probe instead of an… [rocketmq-dashboard]

["Kris20030907 (via GitHub)" <gi...@apache.org>]

Kris20030907 commented on PR #6836:
URL: https://github.com/apache/rocketmq-dashboard/pull/6836#issuecomment-6263490688

   > Thanks — the gap is real: a failed NameServer registry probe currently renders as an empty inventory instead of a failure. Two things block it as-is:
   > 
   > 1. The `id == null` placeholder row has an unhandled consumer. `clients.tsx` picks `nextClusters[0].endpoint`, so an unreachable entry can become the default and `listConnections` then fails against a placeholder. Please filter `id == null` there — or better, drop the synthetic row and carry an explicit `probeFailed` flag instead.
   > 2. This overlaps the registry work in #6816 and #6853. Please rebase once those land.
   > 
   > One convention note: for admin-side queries we only throw when the RPC itself fails; an unavailable inventory should come back as a 200 with an explicit flag rather than as a fabricated row.
   
   Thanks for the review — agreed on all three points. Taking the "better" option: I'll drop the synthetic row entirely and report probe failures explicitly.
   
   Proposed shape for `GET /api/clusters/registry` (an unavailable inventory still returns 200):
   
   ```json
   {
     "clusters": [/* real discovered clusters only */],
     "probeFailures": [{ "name": "dead-ns", "endpoint": "127.0.0.1:9877", "error": "..." }]
   }
   ```
   
   - No fabricated rows, so `clients.tsx` can no longer pick a failed entry as the default endpoint — empty `clusters` again means exactly "probe OK, zero clusters".
   - The NameServer tab will read the failure from `probeFailures` (warning status, diff disabled) instead of inferring it from a placeholder's `id == null`.
   - Happy to reshape this into per-entry `probeFailed` flags if you prefer that form.
   
   Will rebase onto #6816 / #6853 once they land and push the rework on top.
   


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