You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "wu-sheng (via GitHub)" <gi...@apache.org> on 2026/08/27 13:42:08 UTC

[PR] fix(cold-stage): ask for cold where cold is kept, and stop reading on the flip [skywalking-horizon-ui]

wu-sheng opened a new pull request, #141:
URL: https://github.com/apache/skywalking-horizon-ui/pull/141

   ## Why
   
   A review of #140 raised ten findings. Nine were verified against the code before anything was changed — six confirmed outright, three real but narrower than described — and the tenth was mine: `apps/bff/CLAUDE.md` claimed alarms and the entity pickers were not cold-staged while the BFF sent the flag on all three. It described an operational recommendation as though it were implemented behaviour.
   
   ## What
   
   **Cold has a scope, and it is ours rather than the protocol's.** The protocol accepts `Duration.coldStage` far more widely than a deployment should use it. Traces, logs and metrics are the classes worth ageing into cold storage; alarms, the instance and endpoint pickers, events and everything from profiling are small, and they are the first things reached for during an incident. Those routes now send no flag at all — and since `coldStage: true` REPLACES the hot read rather than widening it, sending it there did not broaden the answer, it emptied it. `apps/bff/CLAUDE.md` carries the route table, with the right-hand column as the default: a route sends the flag only because its data is a trace, a log or a metric.
   
   **Flipping the Cold pill reads nothing.** It used to fire a round of its own, which on a cold tier routinely ran past the sixty-second cap — and the sweep that followed selected on `dataUpdatedAt`, which a cancelled query never updates, so it re-queued precisely what the cap had just cancelled. The cap was at its most useless during the outage it exists for. The stage also left the query keys: a key that moved on the flip has no cached entry, so the fetch would have happened regardless of the sweep. The header rides on the next read the page makes.
   
   **A capped round stops the work it started.** The signal reached the four graph routes but not the layer landing, the dashboard batches or the overview, so the browser stopped waiting while OAP finished a fan-out nobody would read — multiplied by the node count on a cluster.
   
   **Two shared caches were racing.** The server timezone and the capability probe were fetched per request, so a round's dozen reads probed in parallel; behind a load balancer a fast success could be overwritten by a slower timeout falling back to UTC, and that answer then served every request for the next minute. Each is read once per expiry and shared. The shared flight carries no caller signal on purpose — whoever arrived first must not cancel a probe the others are waiting on.
   
   **Failures stop being published as facts.** A failed service-catalog read no longer replaces the known-good roster: the rows are kept, marked stale, and a failed read expires in seconds rather than holding an empty catalog for a full minute while the sidebar shows zero services. A landing metric batch that timed out no longer leaves its services sorted to the bottom as though idle, where they could fall out of the top-N and rename the layer's default service — unread now ranks above genuinely-absent, and the response reports that the ranking was decided on partial data.
   
   **A retained comparison series is drawn where its data actually is.** Each entity keeps its last good reading when its own read fails — that is the design, so one soft failure does not blank its siblings — but the retained array was plotted across the CURRENT axis and ran to the right-hand edge beside the fresh ones, inviting a comparison between two different windows. It is now positioned at the buckets it was read for: whatever falls outside the axis is dropped, the buckets it never covered stay empty, and a differing STEP draws nothing at all, because no offset makes those buckets comparable. The endpoint-dependency graph learns its window the same way — it moved out of a `WeakMap` keyed by response identity, which vue-query's reactive proxy and structural sharing both defeat, and into the cache entry itself.
   
   **A layer an administrator removed is not an outage.** The page explained that the template was disabled and pointed at an admin control to re-enable it — one most operators cannot reach, on a page that does not exist under the name it gave — while the refresh history recorded "OAP could not be reached" for an administrative decision on a healthy server. The page now says it is not available, the sidebar drops the layer on the next menu read, and nothing is filed.
   
   **Reactivate restores the layer's own dashboard.** OAP keeps a disabled row's configuration — `disabled` is only a flag on it — and Reactivate was re-pushing the bundled default over the top, discarding every edit the layer carried. It restores what was there, falling back to the bundled default only when there is nothing to restore.
   
   **The cold-stage warning gave advice that could not work.** With no cold stage configured — the BanyanDB default, where every `enableColdStage` ships off — it still said "pick a window older than N days ago", and following it moved the range out of hot+warm so the warning hid itself: a blank page with the one sentence explaining it gone. It now says plainly that no cold stage is configured and stays up until Cold is turned off. Where cold IS configured, the suggested window is the one that clears the deepest class rather than the shallowest; naming the records boundary (3 days by default) brought traces back and left every metric widget, which needs 7, exactly as empty.
   
   ## Validation
   
   Type-check, both builds, root `pnpm lint`, license headers, i18n parity across all 8 locales, source budget, docs links. Unit suites: 837 UI, 1559 BFF, 167 api-client.
   
   Eight new unit tests, each confirmed to fail against the code it replaces: the cold flip starting no round and refetching nothing while still sending the new stage; one shared probe across concurrent callers, and a cancelled caller that neither hangs nor poisons the shared answer; a retained comparison series placed at its own buckets, including the no-overlap and differing-step cases; and the trap banner's two corrected behaviours.
   
   On a real OAP: `core` 11/11 verify cases and `es` 3/3, zero spec failures in both, on an image rebuilt from this branch. The `es` case matters here because the retention page's shape and the backend classification can only be exercised where the backend is not BanyanDB.
   


-- 
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: notifications-unsubscribe@skywalking.apache.org

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


Re: [PR] fix(cold-stage): ask for cold where cold is kept, and stop reading on the flip [skywalking-horizon-ui]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #141:
URL: https://github.com/apache/skywalking-horizon-ui/pull/141


-- 
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: notifications-unsubscribe@skywalking.apache.org

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