Skip to content

api: derive multicast subscriber/publisher counts from live users - #653

Open
armcconnell wants to merge 1 commit into
mainfrom
fix/multicast-live-subscriber-counts
Open

api: derive multicast subscriber/publisher counts from live users#653
armcconnell wants to merge 1 commit into
mainfrom
fix/multicast-live-subscriber-counts

Conversation

@armcconnell

Copy link
Copy Markdown
Contributor

Resolves: #650

Summary of Changes

  • Derive per-device multicast subscriber/publisher counts from live dz_users_current data instead of the stale on-chain multicast_subscribers_count / multicast_publishers_count fields on dz_devices_current. In production those on-chain fields are frequently 0 even when a device has active subscribers, so DZDs showed 0 subscribers and 0 available capacity (appearing oversubscribed) despite real users.
  • A multicast user counts as a subscriber when its subscribers array is non-empty and as a publisher when its publishers array is non-empty; a user that does both counts in each.
  • Applied consistently across the device list/detail, metro list/detail, contributor detail, and facility rollup so the whole multicast section agrees. Aggregate pages sum the live per-device counts, and the effective-max capacity is floored at the live count.
  • No frontend change: the web reads the same JSON fields, which now carry correct values.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 4 +102 / -40 +62
Tests 4 +315 / -0 +315
Total 8 +417 / -40 +377

Small, focused query changes in four handlers; the bulk of the diff is new handler tests.

Key files (click to expand)
  • api/handlers/metros.go — replace on-chain sub/pub counts with a device_multicast_live CTE in both the list (shared const) and detail queries
  • api/handlers/facilities.go — sum live per-device counts in the facility rollup CTE
  • api/handlers/contributors.go — sum live per-device counts in the contributor detail query
  • api/handlers/devices.go — extend the multicast CTE to compute live sub/pub counts for the device list and detail

Testing Verification

  • Added handler tests (test-first) covering a device whose on-chain counts are 0 but which has activated multicast users that subscribe, publish, or both; asserts the API reports the live counts (e.g. 4 subscribers / 3 publishers), and that metro/facility aggregates sum correctly across devices.
  • Pending multicast users are excluded; a both-roles user is counted in both subscriber and publisher totals.
  • Full api/handlers package test suite passes.

The device, metro, facility, and contributor handlers read the on-chain
multicast_subscribers_count / multicast_publishers_count fields from
dz_devices_current. In production these are frequently stale or 0 even when
the device has active multicast subscribers, so DZDs showed 0 subscribers and
0 available capacity (appearing oversubscribed) despite real users (#650).

Derive the per-device subscriber and publisher counts from dz_users_current
instead, counting activated multicast users with a non-empty subscribers /
publishers array (a user that both publishes and subscribes counts in each).
The metro and contributor aggregates and the facility rollup now sum these
live counts, and the effective-max capacity is floored at the live count.

@ben-dz ben-dz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core change is sound: subscribers/publishers are non-nullable String DEFAULT '[]' so countIf(JSONLength(...) > 0) is safe, the device_multicast_live CTE groups by device_pk so joins are 1:1 with no fan-out, and the four changed CTEs are consistent. No blockers. Worth addressing before/with merge:

  • topology handler left on stale on-chain countstopology.go:230-231 still reads d.multicast_subscribers_count/d.multicast_publishers_count, which drive the topology graph's per-device S/P labels. A device will show live counts everywhere except the topology graph (the #650 symptom as a cross-page contradiction). Apply the same CTE there or track a follow-up.
  • contributors omits the live-count capacity floor metros and devices apply — contributors.go:323-324 can emit multicast_subscribers_count > max_multicast_subscribers in the payload; the web clamps but the agent/MCP consumers see an oversubscribed contributor. Wrap eff_max_subs/eff_max_pubs in greatest(if(...), toUInt64(COALESCE(dml.live_subscribers, 0))) to match metros.
  • tests pin "nonzero", not "replace" — all fixtures leave the on-chain counts at 0, so a greatest(onchain, live) or summing implementation would still pass. Add a fixture with a stale nonzero on-chain value (e.g. on-chain 9, live 4, assert 4).

Two lows: the detail-endpoint CTEs scan all of dz_users_current unfiltered per request (uncached; watch given #640/#642); and the toUInt16 cast in devices.go exists only because those two struct fields are uint16 while the other three handlers use uint64 — widen for consistency.

Findings not anchored to the current diff:

  • api/handlers/topology.go:230 — medium: Topology still reads the stale on-chain d.multicast_subscribers_count / d.multicast_publishers_count (line 231) that this PR replaces everywhere else. These fields drive the topology graph's per-device S/P labels and hover panel (web/src/components/topology-graph.tsx:404,3877-3879), so a device will show live counts on its detail/metro/contributor pages but 0 on the topology graph — the #650 symptom re-appearing as a cross-page contradiction. Apply the same device_multicast_live CTE + join here, or track an explicit follow-up. Not a regression (topology was already stale).

toUInt64(max_multicast_subscribers) as raw_max_subs,
toUInt64(max_multicast_publishers) as raw_max_pubs,
if(max_unicast_users > 0, toUInt64(max_unicast_users), toUInt64(greatest(0, toInt64(max_users) - toInt64(max_multicast_subscribers) - toInt64(max_multicast_publishers)))) as eff_max_unicast,
if(max_multicast_subscribers > 0, toUInt64(max_multicast_subscribers), toUInt64(greatest(0, toInt64(max_users) - toInt64(max_unicast_users) - toInt64(max_multicast_publishers)))) as eff_max_subs,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eff_max_subs/eff_max_pubs (lines 323-324) are not floored at the live count, unlike metros (metros.go:121-122,416-417 wrap the same expression in greatest(..., toUInt64(COALESCE(dml.live_subscribers, 0)))) and devices (devices.go:187-188). With a now-live used-count and an on-chain-derived cap, a contributor whose devices have max_multicast_subscribers = 0 and exhausted max_users can return multicast_subscribers_count > max_multicast_subscribers in the JSON. The web clamps via Math.max (contributor-detail-page.tsx:232), but the agent/MCP see an oversubscribed contributor. Wrap both in greatest(if(...), toUInt64(COALESCE(dml.live_subscribers, 0))) / live_publishers.

err := api.DB.Exec(ctx, `
INSERT INTO dim_dz_devices_history
(entity_id, snapshot_ts, ingested_at, op_id, is_deleted, attrs_hash, pk, code, status, device_type, contributor_pk, metro_pk, public_ip, max_users) VALUES
('dev-mc', now(), now(), generateUUIDv4(), 0, 1, 'dev-mc', 'MC-DEVICE-01', 'activated', 'switch', '', '', '10.0.9.1', 100)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture (and the contributor/facility/metro helpers) leaves the on-chain multicast_subscribers_count/multicast_publishers_count at their DEFAULT of 0, so all six new tests would also pass under a greatest(onchain, live) or summing implementation — they pin "nonzero", not the "live replaces on-chain" contract the PR describes. Add one device with a stale nonzero on-chain value (e.g. on-chain subscribers = 9, live = 4, assert 4).

Comment thread api/handlers/metros.go
),
-- NOTE: keep in sync with the identical CTE in GetMetros above
-- NOTE: keep in sync with the identical CTEs in GetMetros above
device_multicast_live AS (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

device_multicast_live scans all of dz_users_current with per-row JSONLength, unfiltered by the requested pk, while the sibling CTE above filters by ?; this detail endpoint isn't page-cached. Marginal today, but given the multicast-health overload history (#640/#642) consider scoping the CTE to the entity's devices in the detail handlers. Same pattern at contributors.go:290.

Comment thread api/handlers/devices.go
-- Live subscriber/publisher counts from attached users. The on-chain
-- d.multicast_subscribers_count / d.multicast_publishers_count fields are
-- frequently stale or 0 even when users are attached (#650).
toUInt16(COALESCE(ucm.subscriber_count, 0)) as multicast_subscribers_count,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toUInt16 (also lines 170, 424, 426) exists only because DeviceListItem/DeviceDetail keep uint16 fields (lines 37,39,335,337) while metros/facilities/contributors use uint64 for the same quantity; toUInt16 wraps mod 65536 rather than erroring. Widen the two device fields to uint64 to drop the cast and align all four handlers.

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.

multicast: DZDs with active subscribers show 0 users and falsely flag as oversubscribed

2 participants