Serve device definitions from the R2 catalog manifest - #188
Open
zer0stars wants to merge 8 commits into
Open
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.
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
1 participant
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.