api: derive multicast subscriber/publisher counts from live users - #653
api: derive multicast subscriber/publisher counts from live users#653armcconnell wants to merge 1 commit into
Conversation
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
left a comment
There was a problem hiding this comment.
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 counts —
topology.go:230-231still readsd.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-324can emitmulticast_subscribers_count > max_multicast_subscribersin the payload; the web clamps but the agent/MCP consumers see an oversubscribed contributor. Wrapeff_max_subs/eff_max_pubsingreatest(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-chaind.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 samedevice_multicast_liveCTE + 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, |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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).
| ), | ||
| -- 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 ( |
There was a problem hiding this comment.
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.
| -- 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, |
There was a problem hiding this comment.
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.
Resolves: #650
Summary of Changes
dz_users_currentdata instead of the stale on-chainmulticast_subscribers_count/multicast_publishers_countfields ondz_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.subscribersarray is non-empty and as a publisher when itspublishersarray is non-empty; a user that does both counts in each.Diff Breakdown
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 adevice_multicast_liveCTE in both the list (shared const) and detail queriesapi/handlers/facilities.go— sum live per-device counts in the facility rollup CTEapi/handlers/contributors.go— sum live per-device counts in the contributor detail queryapi/handlers/devices.go— extend the multicast CTE to compute live sub/pub counts for the device list and detailTesting Verification
api/handlerspackage test suite passes.