Skip to content

Add multi-consumer pause-reason API to SDK root#204

Merged
novykh merged 2 commits into
mainfrom
sdk-pause-reasons
May 19, 2026
Merged

Add multi-consumer pause-reason API to SDK root#204
novykh merged 2 commits into
mainfrom
sdk-pause-reasons

Conversation

@ktsaou
Copy link
Copy Markdown
Member

@ktsaou ktsaou commented May 18, 2026

Summary

The SDK root's paused attribute was being written directly by each
caller — a UI hover toggle, an open modal, a long-running export.
Whichever caller flipped last won, and the "last flipped" was often
the wrong one: e.g., when the cursor moved from a topology canvas
onto an open actor modal, the canvas hover-end fired and wrote
paused: false, releasing the modal's still-active pause intent.
Updates resumed underneath the modal, tables and the mini-topology
jittered on every refresh tick.

The fix is multi-consumer pause-reason aggregation INSIDE the SDK,
exposed on every node via addPauseReason / removePauseReason.
Reasons compose: hover-end removes its own id and leaves the modal's
id in place, the aggregate stays true while the modal is open.

API

  • node.addPauseReason(reasonId) — registers an opaque reason id.
  • node.removePauseReason(reasonId) — removes it.
  • paused is auto-recomputed as
    blurReason || pauseReasons.size > 0 whenever a reason is added/
    removed OR when the blur sources (blurred,
    autofetchOnWindowBlur) change.
  • Idempotent on the same id, no-op for empty/missing ids, fully
    cleared on destroy().

Downstream

@netdata/cloud-frontend's usePauseSDK hook moves from its own
module-level Set to this API in netdata/cloud-frontend#5520. After
that PR lands and the SDK ships a version with this change, the
cloud-frontend hook becomes a thin wrapper that just pushes/pulls
its useId() through the SDK API.

Tests

8 new tests in src/sdk/makeNode.test.js cover the aggregation,
blur combination, idempotency, empty-id handling, and the original
clobber-bug regression (remove-one keeps aggregate true while
another remains).

Test plan

  • yarn test src/sdk/makeNode.test.js passes
  • No behavioral change for callers that haven't migrated —
    they continue to write paused directly via
    updateAttribute (the new API only adds reasons to a private
    Set; direct attribute writes still work the same way)
  • Once cloud-frontend#5520 lands and consumes this, verify in
    the browser: open the actor modal on a topology view, move the
    cursor onto the modal, confirm updates STAY paused

ktsaou and others added 2 commits May 18, 2026 11:38
The `paused` attribute on the SDK root used to be set directly by
each independent caller — a UI hover toggle, an open modal, a
long-running export. Whichever caller flipped last won, and the
"last flipped" was often the wrong one: e.g., when the cursor moved
from the topology canvas onto an open actor modal, the topology
hover-end fired and wrote `paused: false`, releasing the modal's
still-active pause. Updates resumed underneath the modal,
re-fetched data, and the tables / mini-topology jittered every tick.

Fix: the SDK root now owns a private `pauseReasons` Set and exposes
`addPauseReason(reasonId)` / `removePauseReason(reasonId)`. The
`paused` attribute is recomputed as
`blurReason || pauseReasons.size > 0` whenever a reason is added /
removed OR when the blur sources (`blurred`,
`autofetchOnWindowBlur`) change. Reasons compose: hover-end removes
its own id and leaves the modal's id in place, so the aggregate
stays true while the modal is open.

Idempotent on the same reason id, no-op for empty/missing ids,
fully cleared on `destroy()`.

8 new tests in `makeNode.test.js` cover the aggregation, blur
combination, idempotency, and the original clobber-bug regression
(remove-one keeps aggregate true while another remains).

Downstream: `@netdata/cloud-frontend`'s `usePauseSDK` hook moves
from its own module-level Set to this SDK API in a parallel PR.
@novykh novykh self-requested a review May 19, 2026 09:36
@novykh novykh merged commit a1fe455 into main May 19, 2026
@novykh novykh deleted the sdk-pause-reasons branch May 19, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants