FedWiki's site usage is a running total that many code paths adjust; derive it instead #165

New Issue

2026-10-06T05:04:10Z

cgalo5758 commented

2026-10-06 05:04:10 +00:00

The problem

core.numeric_entitlement_usage for the fedwiki_sites key holds one number per pool, shared by every workspace of the organization the pool belongs to. It is the figure the create form compares against the limit. Code in internal/integrations/fedwiki keeps that number as a running total: eleven call sites raise or lower it with AtomicIncrementUsage and AtomicDecrementUsage (workflows/activities.go, workflows/reconcile.go, workflows/swap.go, workflows/sync.go), and usage/usage.go adds three more that raise it to, or bound it between, a count taken from the site rows (ReconcileWorkspace, BoundWorkspace). The paths cover create, delete, archive, restore, keep active, swap, the downgrade sweep and the re-upgrade, the farm sync and the repair that runs at boot.

Each path runs in its own transaction, and some key the counter by workspace while others key it by pool. When two paths overlap, or one keys by the wrong grain, the total ends above or below the number of active sites the pool really has. Above, a member is refused a site they are entitled to. Below, a pool exceeds its limit.

The entitlements model page also says the counter has one mutator, the consumer that increments it. Nothing in the code holds the FedWiki paths to that: the boot repair, the sync and the swap each lower or overwrite the figure.

Why patching does not converge

Each review round of this code found a pair of paths that disagreed and fixed that pair. The next round found another pair. The fixes are correct one at a time, but every new status change in FedWiki adds a path that has to be reconciled by hand with each existing one, and the repair paths (ReconcileWorkspace, BoundWorkspace) exist because the counter drifts. A total that is adjusted from many places cannot be shown correct by reading any one of them.

Proposed direction

Compute the figure instead of maintaining it. Every transaction that changes a site's status first takes a lock on the pool's usage row, then writes the pool's absolute usage, counted from the site rows plus the lifecycle requests still in flight (a create in flight holds a slot). No path increments or decrements; the number written is always a count taken under the lock.

This matches how the integration contract already treats an application's usage, which an application reports as an absolute figure and never as an increment. It also fits the planned lease model for keys the console enforces itself, where usage is committed units plus live leases, still a count.

Open points for the discussion: whether the in-flight requests are read from the workflow tables or recorded as their own rows; whether the same rule is wanted for the other numeric keys; what happens to RaiseNumericUsageTo, BoundNumericUsage and the boot repair once nothing drifts; and the wording of the model page's statement about who may change the counter.

Related: #164 (what an unlimited limit is) changes how the limit side of the same comparison is read; #127 touches how several rules for one key combine into that limit.

Done when

One function writes the fedwiki_sites usage of a pool, and every FedWiki path that changes a site's status calls it under the pool lock.
git grep AtomicIncrementUsage AtomicDecrementUsage finds no call in internal/integrations/fedwiki.
A test runs create, delete, archive, restore, swap and the downgrade sweep concurrently against one pool and ends with usage equal to the active sites.
The entitlements model page states who writes the counter and how.

## The problem `core.numeric_entitlement_usage` for the `fedwiki_sites` key holds one number per pool, shared by every workspace of the organization the pool belongs to. It is the figure the create form compares against the limit. Code in `internal/integrations/fedwiki` keeps that number as a running total: eleven call sites raise or lower it with `AtomicIncrementUsage` and `AtomicDecrementUsage` (`workflows/activities.go`, `workflows/reconcile.go`, `workflows/swap.go`, `workflows/sync.go`), and `usage/usage.go` adds three more that raise it to, or bound it between, a count taken from the site rows (`ReconcileWorkspace`, `BoundWorkspace`). The paths cover create, delete, archive, restore, keep active, swap, the downgrade sweep and the re-upgrade, the farm sync and the repair that runs at boot. Each path runs in its own transaction, and some key the counter by workspace while others key it by pool. When two paths overlap, or one keys by the wrong grain, the total ends above or below the number of active sites the pool really has. Above, a member is refused a site they are entitled to. Below, a pool exceeds its limit. The entitlements model page also says the counter has one mutator, the consumer that increments it. Nothing in the code holds the FedWiki paths to that: the boot repair, the sync and the swap each lower or overwrite the figure. ## Why patching does not converge Each review round of this code found a pair of paths that disagreed and fixed that pair. The next round found another pair. The fixes are correct one at a time, but every new status change in FedWiki adds a path that has to be reconciled by hand with each existing one, and the repair paths (`ReconcileWorkspace`, `BoundWorkspace`) exist because the counter drifts. A total that is adjusted from many places cannot be shown correct by reading any one of them. ## Proposed direction Compute the figure instead of maintaining it. Every transaction that changes a site's status first takes a lock on the pool's usage row, then writes the pool's absolute usage, counted from the site rows plus the lifecycle requests still in flight (a create in flight holds a slot). No path increments or decrements; the number written is always a count taken under the lock. This matches how the integration contract already treats an application's usage, which an application reports as an absolute figure and never as an increment. It also fits the planned lease model for keys the console enforces itself, where usage is committed units plus live leases, still a count. Open points for the discussion: whether the in-flight requests are read from the workflow tables or recorded as their own rows; whether the same rule is wanted for the other numeric keys; what happens to `RaiseNumericUsageTo`, `BoundNumericUsage` and the boot repair once nothing drifts; and the wording of the model page's statement about who may change the counter. Related: #164 (what an unlimited limit is) changes how the limit side of the same comparison is read; #127 touches how several rules for one key combine into that limit. ## Done when - One function writes the `fedwiki_sites` usage of a pool, and every FedWiki path that changes a site's status calls it under the pool lock. - `git grep AtomicIncrementUsage AtomicDecrementUsage` finds no call in `internal/integrations/fedwiki`. - A test runs create, delete, archive, restore, swap and the downgrade sweep concurrently against one pool and ends with usage equal to the active sites. - The entitlements model page states who writes the counter and how.

cgalo5758 added the

kind

design

area/entitlements area/fedwiki labels 2026-10-06 05:04:10 +00:00

cgalo5758 referenced this issue

2026-10-06 05:14:30 +00:00

Nothing looks for dead code, duplication, tangled functions or a value written from too many places #166

cgalo5758 referenced this issue

2026-10-06 18:11:39 +00:00

An application outside the console cannot connect to it: there is no published protocol #171

cgalo5758 referenced this issue

2026-10-06 18:11:48 +00:00

Nothing decides who enforces a numeric limit, or what happens when a connected application loses access #172

cgalo5758 referenced this issue

2026-10-06 18:12:18 +00:00

Nothing looks for dead code, duplication, tangled functions or a value written from too many places #166

cgalo5758 commented

2026-10-06 18:12:18 +00:00

Order: comes after #166 and before #171, the first part of the integration contract. #172 turns this counter into the absolute usage an application reports, so deriving it here is also the first step toward that.

cgalo5758 added this to the Public launch milestone 2026-10-06 18:12:19 +00:00

cgalo5758 referenced this issue

2026-10-06 21:25:45 +00:00

Locks are written out at each call site, with four key conventions and no written order #186

cgalo5758 commented

2026-10-06 21:26:41 +00:00

Three more things the rewrite touches. Tests pay for the running total too: assertCounterMatchesRows and the counter writes in the sites walkthrough go once the figure is derived. The farm sync reads open requests once, without a lock, and deletes farm-absent rows only in the System workspace, so its count and the derived one have to agree on both. And creation should end as one function, with the JSON and HTML handlers only mapping its answer.

Three more things the rewrite touches. Tests pay for the running total too: `assertCounterMatchesRows` and the counter writes in the sites walkthrough go once the figure is derived. The farm sync reads open requests once, without a lock, and deletes farm-absent rows only in the System workspace, so its count and the derived one have to agree on both. And creation should end as one function, with the JSON and HTML handlers only mapping its answer.

cgalo5758 commented

2026-10-06 22:49:03 +00:00

This starts once the static-analysis gate and the test kit land (the first two steps of #166), and runs alongside the rest of that refactor, which touches no FedWiki code. It removes four of the complexity exceptions the gate lists: the three FedWiki functions it rewrites (CreateSite, swapActiveSite, SyncSitesToDBActivity) and TestNumericUsageReconcile, which tests the two counter queries (RaiseNumericUsageTo, BoundNumericUsage) whose future the rewrite decides. It also adds the usage-row lock to the written lock order.

This starts once the static-analysis gate and the test kit land (the first two steps of #166), and runs alongside the rest of that refactor, which touches no FedWiki code. It removes four of the complexity exceptions the gate lists: the three FedWiki functions it rewrites (`CreateSite`, `swapActiveSite`, `SyncSitesToDBActivity`) and `TestNumericUsageReconcile`, which tests the two counter queries (`RaiseNumericUsageTo`, `BoundNumericUsage`) whose future the rewrite decides. It also adds the usage-row lock to the written lock order.

cgalo5758 referenced this issue

2026-10-06 23:00:20 +00:00

Nothing looks for dead code, duplication, tangled functions or a value written from too many places #166

cgalo5758 commented

2026-10-08 07:49:29 +00:00

The order has changed: this now starts once all of #166 has landed, steps 3 to 6 included, instead of alongside those steps. The integration contract (#171 to #174) still follows it.

The reason is the four workflow types that stay registered only for runs the previous release started (CreateFedWikiSiteWorkflow, DeleteFedWikiSiteWorkflow, SetSiteStatusWorkflow, SwapActiveSiteWorkflow). Their activities still adjust the running total, so the rewrite removes them, and removing them is safe on a deployment only once it runs a release that no longer starts them and has no run of them open. Starting after the refactor gives deployments time to get there, and the refactor does not wait on it. If such a run could still be open when this starts, the rewrite adds a check that refuses to start the console while one is, rather than keeping the types.

The order has changed: this now starts once all of #166 has landed, steps 3 to 6 included, instead of alongside those steps. The integration contract (#171 to #174) still follows it. The reason is the four workflow types that stay registered only for runs the previous release started (`CreateFedWikiSiteWorkflow`, `DeleteFedWikiSiteWorkflow`, `SetSiteStatusWorkflow`, `SwapActiveSiteWorkflow`). Their activities still adjust the running total, so the rewrite removes them, and removing them is safe on a deployment only once it runs a release that no longer starts them and has no run of them open. Starting after the refactor gives deployments time to get there, and the refactor does not wait on it. If such a run could still be open when this starts, the rewrite adds a check that refuses to start the console while one is, rather than keeping the types.

cgalo5758 commented

2026-10-09 03:45:19 +00:00

The FedWiki tests now build their fixtures through internal/integrations/fedwiki/fedwikitest, so the tests for this change start from one place. Env.Counted(name, limit, used) gives a tenant's pool a fedwiki_sites limit and a usage row reading used, and Env.Limit and Env.Usage set the two halves on their own. A test sets the figure directly, which is how it reaches usage above the limit, a figure that disagrees with the active sites, or a limit no rule governs. Those are the drifted states a derived figure has to correct, and the concurrent test under "Done when" can start from them.

The swap tests' fixture, swapRequests, records the activation and then the park under one workflow id, in the order the swap handler uses, so those tests read request rows shaped like the handler's. Splitting the swap and sync tests into named subtests left every outcome as it was.

Two tests still insert FedWiki rows by hand: TestAcceptedCreateQueries, and part of TestSyncSkipsAFarmSiteAnOpenCreateHolds.

The FedWiki tests now build their fixtures through `internal/integrations/fedwiki/fedwikitest`, so the tests for this change start from one place. `Env.Counted(name, limit, used)` gives a tenant's pool a `fedwiki_sites` limit and a usage row reading `used`, and `Env.Limit` and `Env.Usage` set the two halves on their own. A test sets the figure directly, which is how it reaches usage above the limit, a figure that disagrees with the active sites, or a limit no rule governs. Those are the drifted states a derived figure has to correct, and the concurrent test under "Done when" can start from them. The swap tests' fixture, `swapRequests`, records the activation and then the park under one workflow id, in the order the swap handler uses, so those tests read request rows shaped like the handler's. Splitting the swap and sync tests into named subtests left every outcome as it was. Two tests still insert FedWiki rows by hand: `TestAcceptedCreateQueries`, and part of `TestSyncSkipsAFarmSiteAnOpenCreateHolds`.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: wiki-cafe/member-console#165