feat(schema): provider_bandwidth table and provider_egress_location verdict columns - #414
Open
Ryanmello07 wants to merge 21 commits into
Open
feat(schema): provider_bandwidth table and provider_egress_location verdict columns#414Ryanmello07 wants to merge 21 commits into
Ryanmello07 wants to merge 21 commits into
Conversation
SetConnectionLocation panicked when a client's IP resolved to a country- or region-only location (no city), because it inserted the location row's NULL city_location_id / region_location_id into network_client_location, where both are NOT NULL. The panic is the real damage, not just a failed lookup: it propagates out of the connection announce goroutine (connect/transport_announce.go), whose HandleError wrapper cancels the connection-level context -- tearing down the entire connect connection right after auth. And because the panic hits before the disconnect-cleanup defer is registered, the connection row is orphaned as connected=true forever. How often this fires depends on the geo database's city coverage, so it is rare with a full commercial dataset and constant with a sparse one. Fix: fall back to the coarsest available granularity so the columns are always non-null -- a country-only location stores its country id for city/region too, keeping the provider locatable at country level instead of crashing the connection. If even the country id is absent, return a clean error (the caller already has a graceful retry path) rather than panic.
The sub-project's binding constraint is that country codes are stored/
compared lowercased, and CreateLocation (network_client_location_model.go)
already enforces this at the model layer via strings.ToLower before writing.
SetProviderEgressLocation wrote e.CountryCode verbatim, so an uppercase code
from an operator-run prober (the raw "US" the real geolocation APIs return)
would silently persist uppercase and violate the invariant that
countryCodeLocationIds and other lookups depend on.
Normalize to lowercase before the INSERT/UPDATE, mirroring CreateLocation's
idiom, and add a test asserting SetProviderEgressLocation("US") round-trips
as "us" via GetProviderEgressLocation.
…ion test TestSubmitProviderEgressLocationCountryOnly only checked the persisted CityConfident flag, which is copied straight from args independent of the location-resolution branch. It never asserted the resolved location's LocationType, so a broken granularity gate (resolving city unconditionally) would slip past all 5 existing tests. Add an assertion that the resolved location is LocationTypeCountry, mirroring the pattern already used by TestSubmitProviderEgressLocationCityConfidentStoresCity. Also assert CityLocationId/RegionLocationId are unset, since a country-granularity row's INSERT never populates those columns (verified against CreateLocation and GetLocation).
Adds POST /network/provider-egress-location, an operator-to-server endpoint (not a client route) that ingests probed provider egress locations. Authenticated by a shared secret from the vault (beta-vault/vault/provider_egress.yml, key ingest_secret) compared with hmac.Equal, not a network jwt. Fails closed: an absent vault resource or empty/missing key disables the endpoint (every request rejected) without panicking the api process at startup or per-request.
… just reject Both committed tests for ProviderEgressLocationSubmit ran with the vault unconfigured, so hmac.Equal never executed - a handler that always returned 401 would have passed the same suite. Extract the memoized secret reader (sync.OnceValue) into a reassignable package var plus a plain readOperatorIngestSecret function, with no change to the read logic or the handler's auth gate, so tests can inject a known secret without racing the existing unconfigured-vault reject tests in the same process. Add a positive-path test (correct secret clears auth and reaches the controller, 400 "Unknown client." for an unregistered id) and a negative discrimination test (wrong secret against a configured vault still 401), plus a direct vault-read test. Document in the vault example that the secret is memoized at first request, so rotating it requires an api restart.
…n parity, cover unprotected branches SetConnectionLocation ran on the connect-announce hot path (every client, every connection, inside a retry loop) with two separate DB round trips before ever reaching the mmdb fallback. Fix review findings against the probed-egress location change: - model.GetFreshProviderEgressLocationForConnection replaces the GetNetworkClientForConnection + GetFreshProviderEgressLocation pair with a single query joining network_client_connection to provider_egress_location. Freshness cutoff is still computed/compared in Go, not SQL now(). Cuts the probed-hit path from 3 DB round trips to 2, and the fallback path from 4 to 3. - The probed path now also runs the ARIN org-vs-country foreign check (against the probed country code), matching the mmdb path's GetLocationForIp, so net_type_foreign no longer gives probed providers an unearned ranking advantage over unprobed ones. Factored into a shared arinForeignScore helper; on any ARIN/ip-parse failure it just leaves NetTypeForeign at 0, never erroring or panicking the hot path. - Added tests for the stale-probed fallback, a probed write error (bad location_id) falling back to mmdb without panicking, and the Hosting/Proxy flag-to-score mapping. Verified the stale-fallback test has teeth by temporarily widening maxAge and confirming it fails. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…oversized submissions, wire up ingest secret Pre-merge fixes from the final whole-branch review of provider-egress geolocation: - Fix the probed-path ARIN "foreign" check: it compared the control ip's ARIN org country against the probed egress country (two different ips), flagging a provider foreign precisely when probing changed the answer. Now matches the mmdb path exactly: ARIN org country of the control ip vs. the mmdb country of that same control ip, restoring probed/unprobed ranking parity. - Reject provider-egress submissions with an empty (post-trim) Country/City/Region instead of silently creating a permanently-blank canonical location row via CreateLocation's dedupe-on-name behavior. - Reject over-long Country/City/Region (>128) or Org (>256) instead of panicking inside CreateLocation on a Postgres "value too long" error. - Make SetProviderEgressLocation's upsert monotonic in observed_at so a replayed older submission cannot clobber a newer stored row. - Replace three production-comment references to the (upstream-absent) design-spec doc path with inline prose, and drop the hardcoded beta vault filesystem path from the ingest-secret handler comment. - Delete GetNetworkClientForConnection (model/provider_egress_location_model.go), a fully superseded, zero-caller helper. - Provision the ingest secret end-to-end: beta-setup.sh now generates beta-vault/vault/provider_egress.yml (guarded the same way as the other generated secrets), and BETA.md documents it plus the api-restart requirement, since the endpoint otherwise ships silently disabled. Adds tests for each behavioral fix; all pass, including TestSetConnectionLocationToleratesCountryOnlyLocation. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
The example config lives in this fork's beta deployment tree, which does not exist upstream. Operators configure the ingest secret through their own vault; the handler documents the resource name and key.
…AssertEqual The three provider-egress-location test files imported github.com/go-playground/assert/v2, which is present in this fork's go.mod but not in upstream's, breaking compilation on this upstream-based branch (no required module provides package github.com/go-playground/assert/v2). Upstream already has an equivalent helper, connect.AssertEqual (from github.com/urnetwork/connect, used throughout model/ and controller/ test files), so no new third-party dependency is needed. This is a mechanical 1:1 swap (assert.Equal(t, v1, v2) -> connect.AssertEqual(t, v1, v2), same argument order and semantics) with no change to test logic or assertions. None of the three files used assert.NotEqual. No go.mod/go.sum changes: go-playground/assert was never present in this branch's go.mod/go.sum to begin with.
… overflow, and other review findings Five review findings on the provider-egress-location PR, each empirically reproduced before being fixed: - A far-future observed_at had no upper bound: it would win the monotonic upsert forever, read as fresh forever, and outlive the taskworker sweep, permanently pinning a provider's location with no API-side recovery. Add MaxProviderEgressLocationSubmissionSkew (5m) alongside the existing MaxProviderEgressLocationSubmissionAge check, and reject observed_at further in the future than that. - provider_egress_location.asn was `int` (Postgres int4, max ~2.147e9); ASNs are 32-bit unsigned (max ~4.295e9), so a real ASN like 4200000000 panicked pgx's arg encoding after a ~78s retry-storm hang. The table is new in this same unmerged PR, so the fix edits the CREATE TABLE migration directly (asn int -> asn bigint) rather than adding a second migration. - Removed a "fix(beta)" fork marker comment from model/network_client_location_model.go, rewritten to be vendor-neutral while keeping the NULL-city/region panic explanation intact. - arinForeignScore's doc comment said its second argument was "the mmdb-resolved country ... or the probed egress country" -- a later commit made both callers pass the mmdb country for ranking parity, so the doc was updated to describe what the code actually does and why. - SetConnectionLocation mapped the probed Mobile flag onto NetTypeVirtual, but IpInfo has no Mobile concept and NetTypeVirtual is only ever set from the ipinfo schema's is_satellite field. This gave probed mobile providers a ranking penalty an identical unprobed provider never takes -- the opposite of the parity this feature promises. Mobile stays in the model/wire contract as metadata but no longer feeds net_type_virtual; Hosting/Proxy keep feeding their scores since they do have direct mmdb-path equivalents (ipInfo.Hosting/ipInfo.Privacy). Tests added: future/within-skew observed_at rejection and acceptance, large-ASN round-trip, and a Mobile-does-not-set-net_type_virtual assertion folded into the existing probed-flags-to-scores test. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
The operator's prober decided what to probe from an in-memory ttl cache, so a restart re-probed the whole provider population and nothing durable recorded what was actually due. The freshness data already exists server-side in provider_egress_location.observed_at; this exposes it so the server owns the schedule. model.GetProviderEgressLocationDue sources candidates from the live provider population (network_client_location_reliability, connected + valid) and LEFT JOINs the egress row, rather than selecting from provider_egress_location. The dominant case is a provider that has never been probed and so has no egress row at all; selecting from that table would return exactly the providers that least need probing and none of the ones that most do. Only providers holding a Public provide key are returned. Probing tunnels through the provider itself, i.e. opens a contract from outside the provider's own network, which a provider without a Public key refuses -- offering one to the prober would burn a probe slot on a guaranteed failure. Same EXISTS filter UpdateClientLocations and UpdateClientScores already apply. The staleness cutoff is computed in Go and bound as a query argument. observed_at is a naive `timestamp` holding utc, so comparing it against sql now() would cast through the session timezone and silently skip a window. GET /network/provider-egress-due is operator-to-server, authenticated by the same X-UR-Operator-Secret shared secret as the ingest endpoint (hmac.Equal against the memoized, fail-closed operatorIngestSecret) rather than a network jwt. Its cutoff is half ProviderEgressLocationMaxAge: at the full max age every location would lapse to the mmdb fallback at the exact moment it became due and stay lapsed until the prober reached it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…t succeed The only thing that moved a provider off the head of the due queue was a successful probe writing provider_egress_location. A provider that connects and holds a Public provide key but always fails to probe -- firewalled egress, dead upstream, anything other than the missing Public key the query already screens for -- never gets a row, so its observed_at stays NULL, so it sorts ahead of every stale-but-refreshable provider on every poll, forever. Five hundred such providers and a prober asking for limit=500 gets the same five hundred dead providers every time; no healthy provider's location is ever refreshed. It fails silently: the endpoint keeps returning a full, plausible-looking batch. The in-memory ttl cache this replaced was incidentally immune, because it marked a provider probed whether or not the probe worked; moving the schedule server-side dropped that protection. Record attempts, not just successes. provider_egress_probe_attempt is a small table keyed by client_id -- the attempt cannot live on provider_egress_location, because the case it exists to handle is precisely a provider with no row there. GetProviderEgressLocationDue now requires both no fresh success and no recent attempt, with a much shorter backoff for attempts (6h) than for success freshness (half ProviderEgressLocationMaxAge, 3.5d): a failing provider should be retried periodically, just not on every poll. Both cutoffs are computed in Go and bound as arguments -- attempt_at, like observed_at, is a naive `timestamp` holding utc, and comparing it to sql now() casts through the session timezone. The prober reports an attempt to POST /network/provider-egress-attempt, on the same operator-authenticated surface as the other two endpoints (X-UR-Operator-Secret, hmac.Equal, the memoized fail-closed operatorIngestSecret). The server timestamps the attempt rather than trusting the prober's clock. Attempt rows are swept by the existing expiry task. This pulls probe_attempt_at/probe_failure forward from the P2 data model in docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md, because P1's schedule cannot function without them. Only those two columns, not the rest of the verdict model. Also adds a client_id secondary sort key, so batch composition under a limit is deterministic rather than plan-dependent -- the whole never-probed population otherwise ties on NULL observed_at. Tests: a provider never probed successfully but attempted seconds ago must not be due, must not take the single batch slot from a stale-but-refreshable provider, and must become due again once the backoff elapses; the same over http via the attempt endpoint. Plus the two coverage gaps review flagged: deleting either the connected or the valid predicate now fails a test, and the handler's staleness cutoff is exercised by a probed (not merely never-probed) provider. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…, never create them SubmitProviderEgressLocation validated a probe-supplied city/region only as trimmed, non-empty and <=128 chars, then handed it to model.CreateLocation. CreateLocation dedupes a city on its exact location_name, so an unrecognised spelling did not fail -- it silently inserted a new permanent row into the shared `location` table and added it to the search index. This is not hypothetical. The prober's consensus stores the winning source's original display string, and the three free geolocation APIs demonstrably disagree on spelling: we observed "Frankfurt am Main (Innenstadt I)" against "Frankfurt am Main" for the same host. "Frankfurt am Main", "Frankfurt Am Main" and "Frankfurt/Main" would each become their own row. Those rows are permanent, they survive a code revert, they feed the location search and the provider list, and there is no cleanup path. An ingest endpoint that can add rows to that table is an endpoint that can corrupt it from outside. A geolocation probe has no business defining the world's cities, so the ingest path now resolves against rows that already exist and creates none. model.MatchExistingLocation walks country -> region -> city, trying an exact, fully-indexed match at each level first (the common case: the winning source usually spells it the way the mmdb import did) and falling back to a normalized comparison -- lowercased with punctuation and whitespace dropped -- so the trivial variants land on the row that is already there. Standard library only: a transliteration/fuzzy-match dependency is a lot of new behaviour to take on for a path whose failure mode is already "use the country". Deliberately conservative -- "Frankfurt/Main" folds to "frankfurtmain", does not match "frankfurtammain", and falls back rather than guessing at the wrong row. When nothing resolves, the submission is stored at country granularity instead of being rejected: country is the granularity this design treats as trustworthy, and losing city precision for one probe beats polluting shared data permanently. The country fallback still goes through CreateLocation -- country rows are keyed on country_code, so a variant *name* cannot produce a second row for the same country the way a variant city name can, and a probe from a country not yet in the table must not be dropped. city_confident now records the granularity actually stored rather than what the probe claimed, keeping the schema's documented invariant intact: location_id is a city row exactly when city_confident is set. Tests: an unmatched city stores country granularity and leaves the location row count unchanged; five spelling/case/punctuation variants of one city all resolve to the single existing row and add none. Before this change the unmatched city took the table 3 -> 4 rows and stored as a city, and the variants took it 3 -> 7. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg (cherry picked from commit 682b29b)
…voids a sort
GetProviderEgressLocationDue scanned network_client_location_reliability,
added two LEFT JOINs and an EXISTS, then sorted on observed_at from an
outer-joined table. That sort cannot use an index: the column being ordered
on does not exist for most of the rows being ordered. At beta's 40 providers
it is free. At 100k it is a full scan plus an unindexable sort on every poll.
Measured on a 100k-provider dataset (30% probed, 5% recently attempted):
before Parallel Seq Scan over all 100k rows, hash-joins both tables,
quicksort of ~94k rows (3.3MB), 2282 shared buffers, 62.9 ms
after pass 1: Index Only Scan + two Merge Anti Joins, no sort,
138 rows touched, 18 shared buffers, 0.34 ms
pass 2: Index Only Scan on the new index, no sort,
350 shared buffers, 1.6 ms
The ordering is what makes the split possible. NULLS FIRST means every
never-probed provider sorts ahead of every probed one, so the result is
always the concatenation of two independently ordered groups:
1. never probed -- the dominant group, which is why the ordering is NULLS
FIRST. Within it observed_at is equally absent, so client_id alone
orders it, and as an anti-join with no outer-joined column in the ORDER
BY it is an ordered index scan over the existing (valid, connected,
client_id) index that stops as soon as the batch is full.
2. stale but probed -- only queried when pass 1 came up short of the limit.
Driven from provider_egress_location, where observed_at is a real,
indexable column.
`attempt_at IS NULL OR attempt_at < $n` becomes `NOT EXISTS (... AND $n <=
attempt_at)`, equivalent because client_id is the primary key of
provider_egress_probe_attempt. Same for `observed_at IS NULL`, whose table
also keys on client_id and whose observed_at is NOT NULL -- the only way that
test is true is that no row exists. The two passes are separate snapshots, so
a client that gains its first egress row between them is screened out rather
than handed to the prober twice; the single statement could not do that.
Verified row-for-row identical to the single statement on the 100k dataset at
limits 0, 1, 2, 50, 100, 1000, 20000, 69000, 69500, 70000, 70500 and 100000
-- the middle four straddle the pass-one/pass-two seam (the never-probed
population is ~70138) and the last exhausts the eligible set at 94088.
New migration adds (observed_at, client_id) on provider_egress_location so
pass 2's predicate and its full ORDER BY, tie-break included, are one ordered
index scan; without it the planner adds an Incremental Sort (936 buffers vs
350). Appended, never inserted -- migrations apply by slice index here, so
editing or reordering an applied one corrupts live databases. Pass 1 needs no
new index.
TestGetProviderEgressLocationDueOrderingIsStableAcrossLimits asserts the full
ordering and then that every limit from 0 past the end returns exactly its
prefix, over a population covering never-probed, stale, fresh,
recently-attempted, stale-AND-recently-attempted and non-public. It passes
against the old single statement as well as the new pair -- it is a
behaviour-preservation test, not a description of the new code -- and fails
against a pass 2 that uses the full limit instead of the remainder, and
against a pass 2 that drops the attempt backoff. The stale-AND-attempted
provider exists because of that second check: without it nothing in the suite
covered the only case pass 2's backoff predicate screens, and the broken
variant passed.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
(cherry picked from commit 37ca6cd)
…a probed location
Two review findings on the provider-egress probe ingest path.
1. The location-name normaliser dropped the cases it exists for.
normalizeLocationName kept accented letters, so "São Paulo" did not fold
onto "Sao Paulo", "Zürich" onto "Zurich", or "Kraków" onto "Krakow" --
every one of those missed and fell back to country. The motivating
example missed too: "Frankfurt am Main (Innenstadt I)" folded to
"frankfurtammaininnenstadti", because dropping "(" and ")" as
punctuation leaves the qualifier's letters in the key.
The fold now NFD-decomposes and drops the combining marks (unicode.Mn),
which covers the whole accent class instead of a hand-rolled é->e table
that would keep missing whatever it forgot. golang.org/x/text was
already a dependency of this module and is now required directly; the
stdlib-only constraint the previous revision was written under belongs
to the prober repo's `geolocate` package, not to the server.
matchLocationName gains a third pass that strips parenthesised
qualifiers from both sides. That pass is the only one that can plausibly
land on the wrong row -- "Springfield (IL)" and "Springfield (MA)" both
reduce to "springfield" -- so it requires the stripped key to identify
exactly one candidate and declines on any ambiguity. Falling back to
country is the safe outcome; guessing is not.
Letters carrying a stroke rather than a combining mark (ł, ø, đ) still
do not fold. That is the documented residual: closing it needs the
transliteration table this deliberately avoids, and the failure mode is
country granularity, not a wrong answer.
The existing variant test only covered foldings the implementation
already handled, which is why it could not catch any of this. It now
carries the qualifier cases, and accents get their own test covering
both directions -- accent on the probe, accent on the stored row --
since which side carries it depends on how the row happened to be
seeded. Both fail against the previous matcher.
2. An unmatched city made a provider worse off than never being probed.
SubmitProviderEgressLocation stores country granularity whenever the
probed city does not match an existing location row, and
SetConnectionLocation then let that probed row win unconditionally over
the mmdb lookup. A provider the mmdb had placed in a city was therefore
demoted to a country row and disappeared from every city filter: being
probed made it less discoverable than being ignored.
This is the common case, not a rare one. Cities are not seeded --
AddDefaultLocations runs with cityLimit = 0 -- so a probed city can only
match rows organic traffic already created.
SetConnectionLocation now resolves the mmdb answer first (an in-process
mmdb/ARIN read, no db round trip) and asks probedLocationPreferred
whether replacing it is actually an improvement. The rule is that a
probe may correct a location but never coarsen it:
- a city-confident probe wins; it loses nothing;
- a probe wins when mmdb has no answer, or only a country;
- a probe wins when mmdb's city is in a DIFFERENT country -- that city
is not more precise, it is precisely wrong, and correcting it is the
entire point of the feature;
- mmdb wins when its city or region is in the SAME country the probe
reports, because the probe then adds nothing but a loss of
granularity.
The reasoning is written out at the function, because the naive reading
("a probe is better evidence, so it always wins") is what produced the
regression.
Three tests cover it: the anti-coarsening case fails against the old
unconditional preference, and the two correction cases fail against a
rule that merely keeps whichever answer is finer.
Fix 1 materially reduces how often fix 2 is reached, but neither
depends on the other.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
transfer_escrow already records real bytes delivered per contract as a byproduct of billing. Deriving throughput from it costs nothing additional and cannot be gamed selectively -- a provider cannot inflate real user traffic without actually being fast for real users. Excludes companion (return-traffic) contracts: a client's return leg settles with the client as destination, which would otherwise misread ordinary users as fast providers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
Appends three migrations and the storage path for a measured bandwidth
figure.
- provider_bandwidth: one row per provider, keyed on client_id, tagged with
the source that produced it (passive aggregation of settled bytes, or an
active sample). A new measurement overwrites the old one -- this is the
current figure a consumer reads, not a history, and keeping every sample
would grow without bound for a value only ever read as "the current one".
Indexed on window_end for staleness scans; single-provider lookups go
through the primary key.
- provider_egress_location gains verdict / verdict_reason / assurance. All
three are additive with safe defaults ('unverified' / '' / 'direct'), so
existing rows read as an unjudged direct probe and every existing reader
of the table is unaffected. Nothing writes a non-default verdict yet.
- StoreProviderBandwidth upserts through the client_id primary key, so both
sources write one figure per provider.
The migrations are appended at the tail of the slice and never inserted:
migrations apply by slice index (`for i := DbVersion(ctx); i < upTo; i++`),
so editing or reordering an applied entry corrupts a live database. Every
statement is IF NOT EXISTS, making a re-run -- or a duplicated merge
resolution -- a no-op.
SetProviderEgressLocation names every column explicitly, which bypasses the
new columns' DEFAULTs, so it normalizes an empty Verdict to 'unverified' and
an empty Assurance to 'direct' the same way it already lowercases the country
code. Without that, the additive-with-safe-defaults claim would be false for
every row the current ingest path writes: it computes no judgement, and Go's
zero value '' is a legal varchar. Both readers are updated -- the plain
getter and GetFreshProviderEgressLocationForConnection, which is the
connect-announce hot path -- so neither returns a zero-valued verdict that
looks like a real one.
Tests: the table and the three columns exist (both failed first against the
un-migrated schema); StoreProviderBandwidth round-trips and a second
measurement replaces the first in place; the write path stores the defaults
for an unjudged submission and an explicit judgement verbatim.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
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.
P2 Task 2 of the provider-probing work: the schema and storage path for a measured provider bandwidth figure, plus the verdict columns the probe-ingest path will write next.
This is a stack, and it shows earlier commits.
maindoes not yet have the provider egress-probing work, so this branch necessarily carries it:provider_egress_location, which this PR'sALTER TABLEtargets),ProviderBandwidth,ComputePassiveProviderBandwidth), which this PR's storage path consumes.Each of those is in its own open PR. The only commit unique to this PR is the last one,
feat(schema): provider_bandwidth table and egress verdict columns. Review that; the rest lands with its own PR.What the new commit adds
Three appended migrations:
provider_bandwidth— one row per provider, keyed onclient_id, tagged with thesourcethat produced the figure (passiveaggregation of already-settled contract bytes, or anactivesample). A new measurement overwrites the old one: this is the current figure a consumer reads, not a history, and keeping every sample would grow without bound for a value only ever read as "the current one". Indexed onwindow_endfor staleness scans; single-provider lookups go through the primary key.provider_egress_location.verdict/.verdict_reason/.assurance— the recorded judgement for a probe. Additive with safe defaults ('unverified'/''/'direct'), so existing rows read as an unjudged direct probe and every existing reader of the table is unaffected. Nothing writes a non-default verdict yet.And
StoreProviderBandwidth, an upsert through theclient_idprimary key, so both bandwidth sources write one figure per provider. The figure is advisory: nothing in provider selection or scoring reads it.Migration safety
Migrations apply by slice index (
for i := DbVersion(ctx); i < upTo; i++). All three entries are appended at the tail of this branch's slice — after P1'sprovider_egress_location_observed_at_client_idindex — and the diff ofdb_migrations.gocontains zero deleted lines, so nothing already applied was edited or reordered. Every statement isIF NOT EXISTS, so a re-run, or a duplicated merge resolution while this stack lands, is a no-op.The one non-obvious change
SetProviderEgressLocationnames every column explicitly, which bypasses the new columns' DEFAULTs. Left alone, the current ingest path — which computes no judgement — would store Go's zero value'', a legalvarchar(16), and the "additive with safe defaults" claim above would be false for every row written after this merge. So the write path normalizes an emptyVerdicttounverifiedand an emptyAssurancetodirect, the same way it already lowercases the country code, and a test pins it. Both full-row readers are updated —GetProviderEgressLocationandGetFreshProviderEgressLocationForConnection, the connect-announce hot path — so neither returns a zero-valued verdict that looks real.Verification
Tests written first and confirmed failing against the un-migrated schema (
provider_bandwidth table does not exist;provider_egress_location missing column "verdict"/"verdict_reason"/"assurance").This commit adds four tests: the two schema-shape ones above, a
StoreProviderBandwidthround trip (a second measurement replaces the first in place), and the write-path default/verbatim test.After the migrations: 18/18
modelprovider-bandwidth + provider-egress tests pass (3 from the passive-bandwidth commit, the 4 new ones, 11 pre-existing egress including P1's due-query suite),TestApplyDbMigrationspasses,go build ./...andgo vet ./...clean.In the
controllerpackage, 20/26 egress + connection-location tests pass. The 6 failures are allSetConnectionLocation*mmdb-fallback cases failing withUnknown schema type: GeoLite2-City— an environment mismatch (the local mmdb file is an ipinfo-schema database), not a regression. Two of the six were run directly against this branch's base with this PR's commit absent -- one pure-mmdb case and one probed-plus-mmdb case -- and both fail identically there; all six share the same signature from the same environment mismatch.go test ./modelas a whole package is blocked by a pre-existing missingsubsidy.ymlin this environment, unrelated to this change — the runs above were targeted-runpatterns.🤖 Generated with Claude Code
https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg