Skip to content

fix(market-worker): limit the Pearl Abyss rate per region, not for the whole deployment - #48

Merged
Hebilicious merged 1 commit into
mainfrom
fix/per-region-pearl-abyss-limit
Oct 14, 2026
Merged

Hebilicious merged 1 commit into
mainfrom
fix/per-region-pearl-abyss-limit

Conversation

@Hebilicious

Copy link
Copy Markdown
Owner

Closes #47

What changed

  • Per-region limit. Each region's PearlAbyssCoordinator consults PA_RATE_LIMITER itself, keyed by its own host (pearlAbyssRateLimitKey(host) = central-market-pearl-abyss:<host>). Regions never share a count, including EU, MENA and RU, whose coordinators share weur. Each region keeps one coordinator at its location hint, so the binding's per-location count is in effect exact per region. A code comment and the docs say so.

  • Allowance removed. PEARL_ABYSS_ALLOWANCE_ADDRESS, the permit request to pearl-abyss-allowance.invalid and the allowance option are gone. So is the 1 s local refusal window (PEARL_ABYSS_LOCAL_REFUSAL_MS): it only existed to spare round trips to the allowance object, and a local binding call has no round trip to spare.

  • Exports. PearlAbyssCoordinatorObject keeps its name, so hosts that adopted fix(market-worker): hold one Pearl Abyss rate allowance for the whole deployment #32 only drop the allowance option.

  • Kept from fix(market-worker): bound the Pearl Abyss coordinator's queue and let one caller's abort end only its own wait #31:

    • the permit is asked before a request queues;
    • the queue is bounded (queue-full → RATE_LIMITED/429);
    • each request has its 30 s deadline;
    • sharing survives a caller aborting;
    • the limiter call has a 5 s bound and fails closed as rate-limited.
  • Kept from fix(market-worker): report Pearl Abyss throttling as 429 and budget refusals as QUERY_BUDGET_EXCEEDED #29: a throttled read is still RATE_LIMITED / 429 with Retry-After, or a stale answer when one is cached.

  • Docs and settings: the limit is stated per region, with the total across the 12 regions: 1,200 per 10 s by default, 240 a minute on the demo. Updated:

    • the README and docs/deploy.md tables;
    • docs/api.md's limits table (new row) and its line on the demo;
    • docs/architecture.md;
    • apps/market/README.md;
    • the env.ts and settings.ts comments;
    • the site's settings label;
    • the deploy-settings.feature wording.

    Setting names and default values are unchanged.

How it was reproduced

I wrote the scenarios first and ran them against the code on main (8988949), before changing it.

  • Headless, market-coordinator/region-rate-limits-are-separate. Each location keeps its own counter per key, as Workers rate limits do. The limit is 1 per period per region. EU spends its limit, then NA and MENA each send one distinct request. It fails on main:
    FAILED 3 requests reach Pearl Abyss and none fails as rate limited — AssertionError: Expected values to be strictly deep-equal:
      [ null, + 'upstream/rate-limited', + 'upstream/rate-limited' - null, - null ]
    
  • Workers runtime, worker-entrypoints-region-rate-limits-are-separate. This runs the real gateway and the bundled market entrypoint, with PA_RATE_LIMITER at 1 per 10 s. An EU read is followed by an NA read. It fails on main:
    FAILED 2 Pearl Abyss requests were sent and both market items carry their Pearl Abyss summary — AssertionError: Values are not strictly equal:
    -   1
    +   2
    
  • The region's own limit still holds. region-rate-limit-holds sends a second EU read, headless; worker-entrypoints-region-rate-limit-holds sends a second ASIA read and gets an uncached 429 with Retry-After. Both pass on main and on this branch.

Tests removed or reworded

  • Removed deployment-wide-rate-allowance, worker-entrypoints-deployment-rate-allowance and local-refusal-window.
  • stalled-permit-refuses now speaks of the rate limiter, not the allowance.
  • abandoned-permit now proves that a read every caller abandoned during its permit is never sent, even when the limiter admits it afterwards.

Performance

A market miss no longer pays a round trip to the allowance object in eastern North America; #47 measured that at about +160 ms from EU and +300 ms from ASIA at the minimum. A permit is now a binding call local to the region's coordinator.

Checks

  • moon run agris:generate: no changes; nothing under packages/pearl-abyss/src changed, so the catalog identity is the same.
  • moon run agris:check: exit 0.

…e whole deployment

Each Pearl Abyss region is its own server, so its rate limit belongs to it.
#32 routed every coordinator's permit through one deployment-wide allowance
object, which let one busy region throttle every other and cost each
distinct market miss a round trip to that object.

- Each region's coordinator consults PA_RATE_LIMITER itself, under a key that
  names its host (pearlAbyssRateLimitKey), so regions never share a count,
  even those whose coordinators share a location (EU, MENA and RU in weur).
  A region has one coordinator at one location hint, so the binding's
  per-location count is in effect exact per region.
- Remove PEARL_ABYSS_ALLOWANCE_ADDRESS, the permit request to
  pearl-abyss-allowance.invalid and the one-second local refusal window
  (PEARL_ABYSS_LOCAL_REFUSAL_MS): a permit is a local binding call now, so a
  flood of reads costs no round trips to spare.
- Keep #31's guarantees (permit before queueing, bounded queue, per-request
  deadline, abort-safe sharing, a bounded limiter call that fails closed) and
  #29's RATE_LIMITED / 429 with Retry-After.
- Docs and settings state the limit per region and the total across the 12
  regions: 1,200 per 10 seconds by default, 240 a minute on the demo.
  Setting names and defaults are unchanged.
- Tests: region-rate-limits-are-separate and region-rate-limit-holds, headless
  and in the Workers runtime, replace the deployment-wide allowance
  scenarios; local-refusal-window is gone and abandoned-permit now proves an
  abandoned read is never sent.

Closes #47
@changeset-bot

changeset-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: debefc3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Hebilicious
Hebilicious merged commit 31f1281 into main Oct 14, 2026
4 checks passed
@Hebilicious
Hebilicious deleted the fix/per-region-pearl-abyss-limit branch October 14, 2026 10:01
Hebilicious added a commit that referenced this pull request Oct 14, 2026
… changes them (#50)

Closes #49

## What changed

- **`.github/workflows/check.yml`:** adds a `deploy` job. It runs after
`check` passes, on a push to `main` or a manual run. It asks moon which
tasks the push affects (`moon query affected` from `github.event.before`
to the pushed commit), then deploys:
  - the **demo** when `infra:demo-deploy` is affected;
- the **docs** when `website:build` or `infra:docs-deploy` is affected,
building the site first;
- nothing when neither is affected. A manual run, or a push with no
usable base, deploys both.

The job uses the `cloudflare` environment and runs one deploy at a time
without cancelling one in progress. It fails with a clear error if a
credential is missing.
- **`infra/moon.yml`:** `infra:demo-deploy` declares the runtime code
the demo's Workers are built from as its `inputs`, and
`infra:docs-deploy` declares its stack files. These lists are the only
path rules: the workflow keeps none of its own.
- **`AGENTS.md`, `docs/deploy.md`:** say that CI redeploys the
maintainer's stages, and which token permissions it needs.

Two traps the workflow avoids, both checked locally:
- With `CI=true`, moon silently skips a `runInCI: "skip"` task, so the
deploy steps run the same `alchemy deploy … --stage demo|docs --yes`
commands directly. The tasks keep `runInCI: "skip"`, so a stray `moon
ci` never deploys.
- Without a terminal, `moon query affected` waits to read changed files
from stdin and hangs, so the workflow closes stdin (`< /dev/null`).

## How it was reproduced

Not a bug. To check the path rules, I replayed the job's decision with
this branch's moon inputs over the last 16 merges to `main`:

```
demo docs 31f1281 fix(market-worker): limit the Pearl Abyss rate per region, not for the whole deployment (#48
-    -    8988949 docs(agris): name an issue in every pull request, reproduce before fixing, and free the demo
-    docs 76c2457 docs: correct the search, limit and namespace claims, and Fig 1's search pigeonhole (#36)
demo docs f76218d fix(gateway-worker): serve GraphiQL under a strict CSP, and send nosniff and no referrer eve
demo docs aaabc17 feat(infra)!: limit each client of a self-hosted gateway to 60 requests a minute by default 
demo docs af0118d fix(gateway-worker): admit each client's market reads exactly, and every request through the
demo docs 92720e0 fix(market-worker): hold one Pearl Abyss rate allowance for the whole deployment (#32)
demo docs 98d50ea fix(market-worker): bound the Pearl Abyss coordinator's queue and let one caller's abort end
demo docs 5672fee fix(market-subgraph)!: hold Market.items to the 100-item batch cap and require Market.catalo
demo docs 3677cbb fix(market-worker): report Pearl Abyss throttling as 429 and budget refusals as QUERY_BUDGET
demo docs 3dd2aee fix: mask market and catalog errors to a closed set, and keep originals out of the gateway's
demo docs 139323a chore: add a security policy, a minimum release age and a pinned, checksummed proto installe
-    docs 28b17a8 fix(website): render Fig 1's hotspots as Astro markup and escape its JSON blocks (#26)
-    docs 526a3b9 feat(website): send security headers from the docs site (#25)
demo docs aa2f4a5 fix(pearl-abyss): cap the Huffman decoder's declared output at 8 MiB (#41)
-    -    ab3e94f docs(agris): fast-forward the main checkout to origin/main after every merge (#46)
```

Website-only merges (#25, #26, #36) deploy only the docs. Process-only
merges (#43, #46) deploy nothing. Runtime merges deploy both, because
the site renders runtime facts (cache windows, limits) through
`@group(agris-sources)`.

This PR changes neither stage's inputs, so merging it deploys nothing.
The deploy itself is first exercised by a manual run once
`CLOUDFLARE_API_TOKEN` is set (see below).

## Credentials

- **Done:** the `cloudflare` environment exists, only `main` may deploy
to it, and it holds `CLOUDFLARE_ACCOUNT_ID`.
- **Missing:** `CLOUDFLARE_API_TOKEN`. The local OAuth sign-ins (Alchemy
and Wrangler) are refused by Cloudflare's token endpoints (`9109
Unauthorized`), and Alchemy's own token command needs the account's
Global API Key. The token has to be created in the dashboard with
Workers Scripts edit, Account Settings read and Secrets Store edit.

## Checks

- [x] `moon run agris:check` passes on this branch
- [ ] In a stack, this pull request holds one concern and its own layer
passes `agris:check` (not a stack)
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.

Pearl Abyss rate limit should be per region, not one allowance for the whole deployment

1 participant