Conversation
…eland The definitions catalog (manifest.json written by the definitions-worker) is loaded into memory and refreshed every minute with an ETag-conditional fetch; a stale snapshot keeps serving if a refresh fails. Replaces the per-query tableland-node proxy. - New DefinitionsCatalogService; TablelandApiService deleted. - devicedefinition repository filters/paginates the in-memory catalog; the manufacturer deviceDefinitions connection is now keyed by manufacturer token id rather than tableland table id. - GraphQL schema unchanged; manufacturer.tableId still served from indexed chain events for legacy consumers. - TABLELAND_API_GATEWAY setting replaced by DEFINITIONS_CATALOG_URL.
The transport-error and non-200 paths already fell back to the snapshot; a 200 with an undecodable body did not, hard-failing all DD queries and re-fetching per request. Now all three refresh failure modes serve stale.
…atalog
Three failures in the refresh path, all of which turned a producer-side problem
into a worse consumer-side one.
A degenerate manifest was adopted unconditionally. A well-formed 200 carrying
{"count":0,"definitions":[]} replaced a good snapshot, and every
deviceDefinition query then returned HTTP 200 with an empty result instead of
an error -- a silent outage that looks like a manufacturer legitimately having
no definitions. The manifest states its own size and we were decoding Count
without ever using it; a mismatch between Count and the definitions carried
means the object is truncated or was rewritten from partial state. Both are now
refused, and a held snapshot is kept.
The manifest was also decoded as one document, so a single malformed
definition failed the whole thing: warm pods froze on a stale snapshot and a
cold pod failed every device-definition query, giving a mixed fleet where the
same query succeeds or fails depending on which pod answers. Definitions are
now decoded individually and a bad one is skipped and counted.
Finally, the catalog URL is trimmed of a trailing slash. Every neighbouring
setting in values.yaml has one, and "//manifest.json" does not match the
worker's exact route -- it 404s, which on a cold pod fails every query.
The degenerate-manifest guard and the per-element decode shipped together and
defeated each other. The guard counted raw elements; the snapshot is built from
decoded survivors. One producer-side type change -- identity returning tokenId
as a string, say, which the worker copies verbatim into every document -- fails
every element, so the guard saw a full manifest, the skip loop discarded all of
it, and byID was replaced with an empty map. Every deviceDefinition query then
404s and every manufacturer.deviceDefinitions returns an empty connection, from
a good snapshot voluntarily thrown away. It is self-reinforcing too: with byID
empty, the stale-serving guards elsewhere in the refresh stop firing.
The Count check was also close to useless. A manifest rewritten from partial
state is internally consistent -- every worker write path sets count to the
length it wrote -- so {"count":1,"definitions":[one]}, the exact catastrophe
this was meant to catch, passed unchallenged. It only ever rejected the
exactly-zero case.
Both are replaced by one proportional check on the decoded definitions: refuse
a manifest carrying less than half of what is already held. Size relative to
the current snapshot is the only signal that survives a producer that always
labels its output consistently.
TrimSuffix removes exactly one, so a URL ending in "//" still produced "//manifest.json", which the worker's exact-match route 404s. The cold-start degenerate path also returned without setting lastFetch, so every request on a fresh pod re-entered ensureFresh, took the write lock and re-downloaded the whole manifest -- serialising every request that touches the catalog behind one mutex for as long as the condition lasted.
… empty The proportional guard refused a shrunken manifest on every refresh, using the same held snapshot each time, so the decision never changed. A legitimate purge was therefore never picked up by a running pod while any pod that restarted adopted it immediately -- the same deviceDefinition query answering differently depending on which replica served it, until every pod was cycled, with no escape but an undocumented rollout restart. A shrink is now refused once and adopted if the catalog still reports the same size on the next refresh. A corrupt or truncated manifest is transient; a real purge is not. An empty manifest is never adopted over a populated catalog, at any number of sightings. That is qualitatively different from a proportional shrink: serving stale keeps every query answering correctly, whereas adopting it makes them all answer successfully and emptily, which nothing alerts on. The threshold also had an integer-division bug -- held*3/4 truncates to zero for a small catalog, so the guard could not fire at all below four definitions. Multiplied instead. Finally, the three remaining cold-start failure returns now set lastFetch. The previous commit fixed one of four, leaving the paths that actually occur -- worker down, bad route, DNS failure -- re-downloading the whole manifest on every request behind the write lock.
…y with a floor Two defects, both introduced by the commit before this one. Rate-limiting a failed refresh through lastFetch made the failure look like success. lastFetch drives the freshness short-circuit, so recording a failure there reported it once and then, for the rest of the interval, answered every deviceDefinition query with "no device definition found" and every deviceDefinitions query with an empty connection -- HTTP 200, no error, on every replica, for as long as the catalog was unreachable. A failed refresh now records lastAttempt and the error separately, so the retry is still rate-limited but callers keep seeing the failure. The proportional shrink guard with its two-strike confirmation is gone. It could not protect the pod that matters most: a cold pod holds nothing to compare against, and that is exactly the pod that adopts a stub manifest published while the catalog is being rebuilt. The confirmation cycle was worse than useless -- every realistic way to produce a short manifest is deterministic, so it reports the same size on the next refresh and gets confirmed, while the one genuinely transient case never reaches the check because the decoder catches it first. It also carried state across unrelated episodes, letting an old refusal authorise an immediate adoption later, and required an exact size match, so a purge whose size drifted was never confirmed at all. Replaced by DEFINITIONS_MIN_COUNT: one number the operator sets, checked on every pod. Prod holds ~17,993 definitions and is floored at 15,000; dev holds ~11,554 and is floored at 8,000. Zero disables it. Refusing an empty manifest over a populated catalog is kept independently of the floor.
Member
|
More review today. Reminding myself. |
elffjs
approved these changes
Sep 11, 2026
ensureFresh built the manifest request on the context of whichever caller happened to trigger the refresh. That refresh is shared: every other caller queues behind the write lock and is served by its result. A client that gave up mid-download therefore cancelled the fetch for everyone. On a cold pod the resulting context.Canceled went through failedAttempt, and the backoff then answered every deviceDefinition query with "failed to fetch definitions manifest: context canceled" for a full refreshInterval without re-fetching, repeating whenever an impatient client arrived first. On a warm pod the same cancel hit the stale-serve branch, which set lastFetch and silently skipped the refresh for an interval. Derive the fetch context with context.WithoutCancel so the caller's cancellation can no longer reach the download, and bound it with a service-level refreshTimeout (30s, matching the client timeout) so a detached refresh still cannot hang. A caller's own cancellation is now never recorded as a failed attempt and never marks the cache fresh; running past the refresh's own deadline still is, so the cold-start backoff engages.
…ages The non-2xx branch of ensureFresh called failedAttempt(err) with the err left over from client.Do, which is nil on that path because the request itself succeeded. lastAttempt was set but lastErr stayed nil, and the backoff condition requires lastErr != nil, so a 5xx from the catalog never rate-limited the retry: every deviceDefinition query on a cold pod re-downloaded the manifest for the length of the outage. The existing cold-start test did not catch it because a fresh 502 on each call still produced an error to assert on. Build the status error before branching and pass that to failedAttempt. A 5xx on a cold pod now sets lastErr and is answered from the backoff for the rest of the interval; a 5xx on a warm pod still serves the held snapshot.
4 tasks
zer0stars
force-pushed
the
r2-device-definitions
branch
from
September 14, 2026 23:14
226875d to
4afc3e8
Compare
DEFINITIONS_MIN_COUNT was an int field, and the shared settings loader sets an int to 0 when strconv.Atoi fails, then lets a later field overwrite the error. A value such as '15,000' or '15000 ' therefore loaded as 0 with err=nil and silently disabled the floor; negative values were accepted too. DEFINITIONS_CATALOG_URL was only trimmed of trailing slashes, so a quoted Helm value with a trailing space or newline reached the pod intact and every device-definition query failed to build its request. That error was returned bare, skipped failedAttempt and was never logged. Hold DEFINITIONS_MIN_COUNT as a string and parse both settings in Settings.DefinitionsCatalog. The floor must be empty (0) or digits only. The URL must contain no whitespace or control characters, parse with an http or https scheme and a host, and carry no query or fragment. main validates them before connecting to the database and logs the effective URL and floor at Info. NewDefinitionsCatalogService now returns the validation error and NewResolver passes it on, so construction fails startup as well. A request that fails to build is wrapped, logged and recorded like any other failure. The graph suites construct a resolver, so they now set a catalog URL (http://definitions.invalid, never contacted: they issue no device-definition queries) and check the error. settings.go is gofmt-clean again.
Three defects, all in how a manifest became a snapshot.
The per-element decode only rejected type errors, so null, {} and objects
missing an id or a manufacturer token id decoded into zero-value definitions.
They counted toward DEFINITIONS_MIN_COUNT, cleared the empty guard and the
all-failed guard, and were indexed as byID[""] and byMfrToken[0], so
{"definitions":[null,null,null]} replaced a good snapshot with nothing and
reported success.
The manifest was parsed three times: the whole body into []json.RawMessage, a
json.Unmarshal per element, then an UnmarshalJSON that re-decoded the entire
definition to tolerate "" metadata. On the live 17,993-definition manifest
that cost 139 ms, 57 MB and 639k allocations per refresh, on every pod, every
minute, under a 250m CPU limit in dev.
count and updatedAt were still decoded strictly although nothing has read
count since dd4bf7a, so a producer type change in either would fail the whole
manifest and undo the per-element isolation.
Decode with json.Decoder Token/More/Decode, element by element, skipping every
manifest member except definitions. The "" and null metadata tolerance moves
onto the metadata field's own UnmarshalJSON, so a definition decodes in one
pass. An element that fails with a type error still costs only itself; a
syntax error or truncated body still fails the manifest.
invalidReason names what cannot be served (no id, no positive manufacturer
token id, no slug) and catalogCandidate counts those elements without adopting
them. A catalog whose invalid elements exceed max(10, 1% of elements) is
refused outright, and the floor, the empty guard and the all-invalid guard now
count valid definitions only.
Measured on the live manifest, Apple M4 Max, -count=5:
previous 138.9 ms/op 57.06 MB/op 639,125 allocs/op
streaming 67.0 ms/op 28.34 MB/op 361,325 allocs/op
The new decode reproduces the old one element for element on that manifest
(17,993 identical, 0 skipped by either) and on an edge-case fixture covering
"" , null, {} and typed-error metadata; TestCatalogStreamingDecodeMatches-
ThePreviousDecode keeps the previous decode around to check it.
TestCatalogRejectsDegenerateManifest's "count disagrees with the payload" case
passed for the wrong reason: {"count":9000,"definitions":[]} is refused by the
empty rule, and Count has not been read since dd4bf7a. It is split into
TestCatalogRefusesAnEmptyManifestOverAHeldCatalog, which says the empty rule
is what refuses both cases, and TestCatalogIgnoresManifestCountAndUpdatedAt,
which asserts the rule that does exist: a short manifest with a disagreeing
count and a retyped updatedAt is adopted, and only the floor refuses it.
ensureFresh held the exclusive lock across the download, the 7.7 MB decode and the index rebuild, and the live worker never answers 304 (readThrough ignores If-None-Match and Cloudflare's gzip weakens the stored ETag), so every refresh was a full download that blocked every device-definition read on the pod: 0.4-0.8s of lock per minute, 30s when the origin stalled, and readers served after their own deadlines had passed. A cold pod was worse. Nothing warmed it, so the first query paid for the load, and one transient failure was cached by the backoff and returned to every device-definition query for a full interval with no retry. The Tableland client this replaced recovered from the same 502 in about a second. A warm pod was worse in the other direction. Every refresh failure, including a permanent 404, an undecodable manifest and a floor refusal, set lastFetch, returned nil and left lastErr nil, so warm replicas served arbitrarily stale data with no signal while any restarted replica failed every query: the same query answering from one replica and erroring on the other, indefinitely. The five copy-pasted stale-serve tails also drifted. Two of them stored a raw error and returned a wrapped one, so only the first caller got context and everyone else got a bare "unexpected EOF" or "context deadline exceeded". Refresh on one goroutine, started once through sync.Once, and swap an immutable catalogSnapshot through an atomic.Pointer. Readers load the pointer: no lock, so no reader can ever queue behind a refresh. main calls Resolver.StartBackground before the server listens, so a pod warms before it takes traffic; a first read starts the loop too, so a caller that never starts it still works. A failed refresh retries after a second, doubling with jitter up to the refresh interval, instead of waiting out an interval. A pod with no snapshot waits for the attempt in flight, bounded by the caller's context and by coldWait (10s), then reports the failure rather than hanging the query. refreshOnce is the single failure path: it records the wrapped error every caller and the log then share, counts consecutive failures (Warn, and Error from the fifth), and publishes three metrics on the default registry: identity_definitions_catalog_definitions, _snapshot_age_seconds and _refresh_failures_total, each labelled by source. New setting DEFINITIONS_MAX_STALENESS, default 24h, "0" to disable, rejected if shorter than the refresh interval. Past it, reads fail with an error wrapping the latest refresh failure, so warm and restarted replicas converge on the same answer instead of disagreeing. Both chart values files and the sample settings set it explicitly. Tests: the two "caller disconnects mid-refresh" cases are gone, because a caller no longer triggers a refresh at all; what they protected is now covered by TestCatalogRefreshRunsOnItsOwnDeadline (the refresh's own deadline, and a read that never fetches) and TestCatalogReaderIsNotBlockedByAStalledRefresh (50 reads against a stalled origin, bounded). TestCatalogBacksOffAfterAColdStart- ServerError asserted exactly one fetch for a whole interval, which was the defect; TestCatalogColdStartRecoversFromATransientFailure asserts recovery in seconds instead. The rest drive refreshes explicitly through refreshOnce, so no assertion races the loop. go mod tidy: goqu, httpmock and their transitive retry-go and x/time go, since this branch deleted their last importers; prometheus testutil arrives for the metrics test.
The only source was <base>/manifest.json, a route definitions-worker #1 deletes without a replacement. Its cutover deploys the worker before identity, and worker deploys are manual, so the ordering is an operator's to get right: once the new worker is out, warm pods take the stale-serve path every minute and freeze on a snapshot that no template edit can reach, and the first pod restarted afterwards fails every device-definition query. Nothing here guarded against that or migrated away from it. Ask <base>/idx/current.json first, conditionally. 200: the deployment publishes template builds. A build id equal to the one held means nothing to do -- shards are immutable -- so an unchanged catalog costs one small conditional GET a minute instead of a 7.7 MB download. A new build is fetched shard by shard, eight at a time, and each Template is flattened the way device-definitions-api flattens it (template-level attributes only, sorted by name, strconv-rendered, manufacturer from the template, no ksuid), so both services describe a template identically. A build is vetted whole: a half-empty shard is a catalog that lost definitions, not a shard to serve around. Shard keys are matched against idx/<build>/shard-<n>.json before they are fetched, because the index is data from the network and must not name a URL of the producer's choosing. 304: unchanged. 404: no index deployed, so the flat manifest is read exactly as before. Anything else: a refresh failure. A 5xx must never be read as "this deployment has no index" and flip a pod to the other source. A listing in template mode only changes when a build is published, so a definition created since then is missing from the snapshot. A by-id miss falls back to <base>/t/<id>.json, which the worker writes when the definition is created: found is cached (1024 entries, cleared when a build is adopted), missing is remembered for five minutes (4096 entries), one fetch per id serves every concurrent caller, and the fetch outlives the caller that started it. Any other status is a wrapped error. DefinitionsByManufacturer stays snapshot-only and says so: a listing is a page of results, and fetching it from the origin would put the catalog back in the request path. ToAPI now leaves legacyId null when the ksuid is empty. Templates have none, the schema field is nullable, and &"" answers with a legacy id that resolves to nothing. The devicedefinition fixture called require.Equal inside its httptest handler, which runs on a server goroutine: a path it did not expect would call runtime.Goexit there, kill the connection without writing a response, and surface on the test goroutine as an unexplained EOF. It now routes by path (404 for the index so these tests exercise the legacy manifest), records anything unexpected, and asserts on that from the test goroutine.
Definitions are grouped by the document's manufacturer token id and looked up by the token id in identity's own database, but the catalog carries no chain marker and nothing ties DEFINITIONS_CATALOG_URL to DIMO_REGISTRY_CHAIN_ID. The old _<chainID>_<tableID> Tableland table name made a mismatch fail loudly; reading a flat manifest, it is silent. 108 of the 123 slugs dev and prod share have different token ids. A dev deployment (chain 80002) pointed at https://definitions.dimo.org answers manufacturer(by:{slug:"dodge"}){deviceDefinitions{totalCount}} with three dfsk_glory_* definitions and no error: Dodge is 32 in dev, which is DFSK in prod. settings.sample.yaml pairs local chain 31337 with the dev catalog, and prod pointed at the dev catalog is refused only by accident, because 11,554 happens to fall under the 15,000 floor. Before a candidate is adopted, compare the (token id, slug) pairs it carries with the manufacturers this deployment has. Only token ids present in both are compared: a manufacturer this chain does not have says nothing about which chain the catalog belongs to. Past max(2, 5% of those token ids) the catalog is refused and the mismatches are logged; a couple is drift, logged at Warn and adopted. A lookup that fails logs Warn and adopts: the database is this check's second opinion, not the catalog's source, and a database blip must not stop the catalog from updating. The lookup is injected, so the service still has no database dependency of its own. graph.NewResolver wires manufacturer.Repository.SlugsByTokenID, which is one indexed read of a table of a few hundred rows per adopted catalog.
The opt-in e2e test told you to run it against "local wrangler dev or deployed worker" and then hard-coded prod data, so the dev catalog it also points at failed: Dodge is token 33 on prod and 32 on dev, and dev's BMW carries 997 definitions against an assertion of more than 1500. Choose the expectations from the host: prod, dev, or shape only, which is what a local bucket or a preview deployment needs. Everything else is checked structurally on every host -- sorted ids, a positive manufacturer token id and a slug on every definition, and an id that does not exist answering nil without an error, which in template mode exercises the by-id fallback's 404 path against the live worker. Run today against both deployed catalogs, each of which still answers 404 for /idx/current.json and is read through the legacy manifest: CATALOG_E2E_URL=https://definitions.dev.dimo.org PASS (0.53s) CATALOG_E2E_URL=https://definitions.dimo.org PASS (0.70s)
Freshness was keyed on the build id alone, on the premise that shards are immutable so an unchanged id means an unchanged catalog. Only the shards are immutable. assembleManifest rewrites idx/<build>/manifest.json and idx/current.json on every publish of that id, and definitions-worker src/idx.ts says so outright: "Not write-once, so not immutable ... a publish repeated after more pages landed names more shards." That is a sequencing mistake the worker tolerates: publish build B before the walk reports done, finish the walk, publish B again. The second publish names all ~110 shards under a new createdAt. identity saw the same build id, called confirm() and fetched nothing, serving the short catalog of the first publish indefinitely -- with lastSuccess reset on every refresh, so no staleness bound, log or metric flagged it. The index now decodes createdAt and the shard list as part of the build's identity, carried on the snapshot as a catalogOrigin. The held build counts as current only when the id, the createdAt and the shard list all match; anything else is a publish this pod has not read and is loaded. Shard order is deterministic -- assembleManifest sorts numerically before writing -- so comparing the ordered lists compares the sets. A republished build also invalidates the by-id fallback caches, which were keyed on the build id changing: a definition the short catalog had learned was missing is in the longer one.
DEFINITIONS_MAX_STALENESS was compared against lastSuccess, which confirm() resets on every 304 and every unchanged build. It therefore measured only whether idx/current.json is reachable. A worker whose scheduled rebuild has been failing for three weeks keeps serving the same index perfectly, so every refresh succeeded, snapshot_age_seconds read near zero, and every replica served three-week-old definitions with nothing to alert on. The build's own createdAt was on the wire and deliberately discarded. The index's createdAt is now parsed and carried on the snapshot, and there are two bounds because there are two failures: DEFINITIONS_MAX_STALENESS (24h, unchanged) -- this replica cannot reach the catalog. Restarted by confirm(), as before. DEFINITIONS_MAX_BUILD_AGE (new, 72h) -- the data itself is old, however reachable the catalog is. Not restarted by confirm(): nothing republished the build. The new bound has to be the looser of the two. The worker rebuilds when the live build is 23h old and the walk it then runs takes about two hours, so a healthy catalog routinely serves a build 25h old; 72h is about three missed rebuilds. A configured value below 26h is refused at construction rather than failing every query on a deployment working as designed. identity_definitions_catalog_build_age_seconds is the metric, alongside the existing snapshot_age_seconds, so the two ages are distinguishable before queries start failing. No series is published when there is no build timestamp. Legacy mode keeps the reachability bound alone: the flat manifest's updatedAt moves when someone writes a definition, not when the catalog is rebuilt, so a quiet week and a dead producer are indistinguishable. A createdAt this cannot parse is logged and leaves the build unbounded rather than refusing a catalog whose shards read perfectly.
A 404 on /idx/current.json falls back to /manifest.json, a route this same migration deletes from the worker. Between the worker deploy and the first hand-run publish both answer 404, and a cold pod then had no catalog at all: every deviceDefinition and manufacturer.deviceDefinitions query failed, and what it reported was "definitions catalog returned 404 for manifest" -- a message that names neither URL, reads like a transient origin error, and was logged at Warn for the first four occurrences. The fallback stays: it is the real source until the cutover, and it is what tells "this deployment still publishes only the flat manifest" apart from "nothing is published at all". But the double 404 is now one error that names both URLs and the likely cause, and it is logged at Error from the first occurrence, because no pod in the deployment can load a catalog until someone publishes a build. The cause is an ordering the worker's runbook already fixes: the cutover publishes and smokes a build in Phase 2, before the Go services deploy in Phase 3 (definitions-worker docs/superpowers/2026-09-14-trim-template-cutover.md). The error cites it, so the message points at the step that was skipped.
GetDeviceDefinitions re-derived every definition's manufacturer from its id
prefix and issued a .One() per distinct slug. .One() reports a miss as
sql.ErrNoRows, which the loop returned, so a single definition whose prefix
matches no manufacturer row failed the entire page: manufacturer(tokenId: N)
{ deviceDefinitions(...) } returned nothing at all, for every definition in
it.
That miss is reachable by design. The catalog's idea of a manufacturer's slug
and this database's can differ, and checkChain deliberately warns and adopts
when up to 5% of the manufacturers shared with this chain carry a different
slug -- precisely the drift that makes an id prefix unresolvable.
The page's slugs are known before any of them is resolved, so they are now
fetched in one query keyed by slug, and a slug with no row yields a definition
with no manufacturer -- which the schema already allows and ToAPI already
handles -- logged once per page, naming the slugs.
The lookup splits into three pieces so the decision is testable without a
database: manufacturerSlugs (which slugs a page names), the query, and
indexManufacturers (which of them came back). Only the query itself now needs
a container.
A manufacturer token id is required, and the producer side enforces it, so a stored template without one is bad data. It was treated as two different things depending on how it was asked for: a listing skips it and logs, while a by-id lookup turned it into "template %q cannot be served" -- an error the resolver surfaces as a GraphQL 500, blamed on a caller who asked for an id perfectly correctly. fetchTemplate now answers not-found for a document that fails validation, and logs a Warn naming the id and the reason so the document gets fixed rather than quietly vanishing. The id is remembered in the negative cache for its TTL, like a 404, so a client polling for it does not refetch on every query. The invalid-element rule and the refusal threshold are unchanged: a token id is still required, and a catalog where too many templates lack one is still refused.
refresh()'s 404 branch called refreshLegacy unconditionally: it never looked at what the pod already held. adopt() then vetted the candidate on count alone, and the flat manifest carries roughly 18,000 valid definitions, which clears DEFINITIONS_MIN_COUNT and the no-shrink guard comfortably. So after the cutover, one 404 on /idx/current.json was enough to talk a pod serving a template build back down to the pre-cutover layout. That 404 is not rare. The index is served with a minute of max-age and an R2 get of it can return null while a publish rewrites it, while /manifest.json can still answer 200 from an edge that cached it before the route was deleted -- the cutover runbook's probe table records exactly that state on prod and dev today. What the swap costs is invisible afterwards. GetDefinitionByID turns its by-id template fallback off in legacy mode, so a definition created since the build stops being reachable at all. tooOld stops measuring, because legacy mode has no build timestamp for DEFINITIONS_MAX_BUILD_AGE to bound. And unreachable restarts on every successful legacy refresh, so the reachability bound reads healthy too. One "adopted a definitions catalog snapshot" line with source=legacy was the only signal. The fallback now bootstraps and only bootstraps. adoptedTemplate is a sticky flag set when a template build is adopted, and while it is set a missing index is reported as the failed refresh it is, naming the URL that answered 404 and the build being kept. The manifest is not fetched at all on that path, so nothing can be adopted behind the held build's back. The failure is recorded like any other, so it backs off, escalates to Error after five in a row, and -- if the index really has gone for good -- DEFINITIONS_MAX_STALENESS turns it into failing queries rather than quietly wrong ones. A deployment deliberately rolled back to a worker that serves only the manifest is one pod restart away from reading it again, and the message says so. A pod that holds no snapshot is unaffected: it still reads the flat manifest, which is the real source until the cutover and the state every pod starts in.
lookups.found was a plain LRU with no expiry, while lookups.missing carried
an explicit deadline that was re-checked on every lookup. Both were cleared
only in adopt(), and only when a different build was adopted, so a found
entry survived until the next published build -- up to 23 hours.
That is the wrong lifetime for what this cache holds. The fallback only ever
fires for a definition newer than the current build, which is exactly the
definition a curator has just created and is most likely to correct. Create
ineos_grenadier_2026 at 09:00; a deviceDefinition(by: {id}) query at 09:01
misses the snapshot, fetches /t/ineos_grenadier_2026.json and pins v1; at
09:05 the curator fixes the model name and the worker writes v2 and purges
the CDN -- a purge whose stated reason is this fallback. Every replica that
had answered once kept serving v1 for the rest of the build, and replicas
that had not served v2, so the same query answered differently depending on
which pod took it.
A found entry now carries an expiry like a missing one, and is refetched once
it passes. catalogFoundTTL is one minute: that is the refresh interval, so a
correction reaches a replica about as fast as a new build would, and the cost
is at most one request per id per replica per minute on a path that fires
only for the handful of definitions newer than the build -- served by a CDN
edge, and only reaching R2 just after a purge. It is deliberately no longer
than catalogMissingTTL, because the two kinds of staleness are not
symmetrical: a stale negative answer resolves itself the moment the caller
asks again, while a stale positive one is served as fact.
Clearing both caches on a new build stays as it was: a definition deleted
since must not survive a rebuild, whatever the TTL says.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the per-query tableland-node proxy with an in-memory catalog:
DefinitionsCatalogServiceloadsmanifest.json(written by definitions-worker, ~18k definitions / 7.4MB) and refreshes it every minute with an ETag-conditional fetch. A failed refresh keeps serving the last snapshot.deviceDefinition/manufacturer.deviceDefinitionsresolve from RAM — no per-request network hop.deviceDefinitionsconnection is keyed by manufacturer token id instead of tableland table id, so new manufacturers work with zero provisioning.manufacturer.tableIdstill served from indexed chain events for legacy consumers.TABLELAND_API_GATEWAY→DEFINITIONS_CATALOG_URLin settings/charts."",{},{"device_attributes": null}).Deploy any time after the prod backfill (this is a read-only consumer of the manifest); no coordination with other services needed.
Testing
CATALOG_E2E_URL=... go test -run TestCatalogE2E) passed against a full local backfill of all live Tableland rows.Fixes from the max review (2026-09-14)
/manifest.json, so the catalog now asks/idx/current.jsonfirst and reads that build's shards, flattening each template exactly as device-definitions-api does. A 404 there falls back to/manifest.json, so this deploys safely on either worker. Any other status is a refresh failure, so a 5xx can never flip the source. In template mode a listing only changes when a build publishes, so a lookup that misses the snapshot falls back toGET /t/<id>.json, with small found and missing caches cleared on each new build.DEFINITIONS_MAX_STALENESS, default 24h, turns an indefinitely stale snapshot into an error, so a warm replica and a restarted one stop disagreeing. Consecutive failures log Warn then Error, and/metricsgains the definition count, snapshot age and refresh failures, labelled by source.DEFINITIONS_MIN_COUNTused to load as 0, disabling the floor silently. It andDEFINITIONS_MAX_STALENESSare now parsed and validated, with the effective values logged, and a malformed catalog URL fails startup instead of failing every query with a bare parse error.Deploy dependency
This PR is no longer independent. It deploys in one window with device-definitions-api #314, and only after definitions-worker's
scripts/tableland-delta.mjsreports zero importable missing ids against the catalog this service will read. Prod R2 has been frozen since the 2026-08-25 backfill while prod dd-api still writes only to Tableland, so deploying this first would drop definitions that resolve today.