Give each S3 reporter its own credentials and endpoint, and test against rustfs - #341
Merged
Merged
Conversation
hpr_packet_reporter and hpr_gateway_liveness_reporter upload to different
buckets, but both resolved credentials through aws_credentials -- a single
VM-wide gen_server holding exactly one credential set. That cannot serve a
deployment where each bucket has its own user/password, which is what moving
off AWS requires.
Both reporters now share hpr_s3_client, which resolves one of two targets: an
aws_endpoint (an S3-compatible provider, authenticated with that reporter's own
aws_access_key_id/aws_secret_access_key), or, with no endpoint set, real AWS S3
in aws_region using the aws_credentials provider chain. Clients come from the
aws_client constructors rather than hand-built maps; only proto is patched,
since make_local_client/4 is the sole constructor accepting a custom endpoint
and it pins http.
This also fixes a latent crash in the liveness reporter, which destructured
`token :=` unconditionally and so badmatched on any credentials lacking a
session token. EC2 instance credentials carry one, so the current AWS
deployment was unaffected, but it would have failed on the first report made
with static credentials.
Localstack is now just another endpoint, which retires the <<"local">> region
special case and the four HPR_*_LOCAL_HOST/_LOCAL_PORT variables in favour of a
single HPR_TEST_S3_ENDPOINT. Both reporter suites now actually execute: CI
classified only the packet suite as an AWS suite, and `make test-aws` set a host
but no port, so every case silently skipped.
Also deletes config/sys.config, which no runtime path loads and which had
drifted out of sync with the reporter config.
BREAKING: aws_bucket_region is renamed to aws_region. Deployments must rename
the key in the same change, or the reporters raise {missing_s3_config,
aws_region} at init.
The reporter suites exercised S3 against localstack. Move them to rustfs, so CT runs against the same S3 implementation as oracles-iot, and fold the test backend into the top-level compose file. docker-compose.yaml now contains only rustfs and an init container that creates test-bucket, and docker-compose-ct.yaml is gone. NOTE: this drops the hpr, promtail and prometheus services that docker-compose.yaml used to define, so it no longer runs HPR itself -- it exists purely to back the test suites. rustfs does not implement the batch DeleteObjects API: aws_s3:delete_objects/3 returns an error against it, which left objects behind and made whichever suite ran second see a dirty bucket. Both teardowns now share an empty_bucket/2 helper that removes one key at a time via delete_object/4. That also fixes a pre-existing fragility in the packet suite, which hard-matched <<"Contents">> and so crashed in teardown against an already-empty bucket. The reporters themselves are unaffected; they only ever call put_object. The suites no longer skip themselves when no endpoint is configured -- the endpoint lives in ct.config, and a missing backend should fail rather than silently pass. Both `make test` and `make test-aws` therefore bring rustfs up around the CT run. HPR_TEST_S3_ENDPOINT is retained solely so CI, which runs the suites inside a container where rustfs resolves by service name rather than on localhost, can override that one value.
The reporter suites override aws_endpoint from HPR_TEST_S3_ENDPOINT so CI,
which reaches rustfs by service name rather than on localhost, can point them
at the right host. That override was silently dropped on the first test case
of a run: hpr is not loaded yet at that point, so application:get_env/3
returned its default and the merge produced a config carrying only
aws_endpoint, and starting the app then loaded it and overwrote everything
with ct.config.
The first case therefore ran against ct.config's localhost endpoint, which in
CI refuses the connection. Its end_per_testcase then crashed before
test_utils could stop hpr, so the next case inherited the still-running
application and failed the same way -- both suites failed with
{badmatch,{error,econnrefused}}.
Load the application before reading and writing its env. This went unnoticed
locally because ct.config and the override held the same value, so losing the
override made no observable difference.
Also log the resolved endpoint at reporter startup. Only the bucket and
credential source were logged, which is why the CI output gave no indication
of which endpoint had actually been used.
Renaming aws_bucket_region to aws_region, and the matching
HPR_*_AWS_BUCKET_REGION variables to HPR_*_AWS_REGION, forced every
deployment to change its configuration in lockstep with this branch: the old
key raises {missing_s3_config, aws_region} at init on the new build, and the
new key fails the old build's init match, so there was no rollback-safe
ordering.
The name was cosmetic and not worth that. Restore both the config key and the
environment variable names so existing terraform-managed configuration keeps
working untouched, and this branch can be deployed and rolled back on its own.
michaeldjeffrey
approved these changes
Aug 24, 2026
macpie
added a commit
that referenced
this pull request
Sep 2, 2026
…rs (#342) * Index devaddr ranges by bucket instead of caching lookups Finding the routes for a DevAddr ran a linear ets:select over every devaddr range. A per-DevAddr result cache hid that, but only for addresses it had already seen; every other packet paid the full scan. Measured on a production node before this change: a miss scanned all 213,969 ranges and took 35.4ms, ~40 times a second. find_routes had accumulated 380.5 hours over 239.8 hours of uptime -- 1.59 cores burned continuously -- with at least 73.7% of that time in its >10ms buckets. The cache had grown to 4.35M rows / 744MB and still missed 8.2% of lookups. Its 24h eviction sweep against a 12h TTL is also what produced the daily CPU spikes, since half a day of entries were dropped at once and had to be re-learned a full scan at a time. Replaces it with a bucketed interval index derived from the range table, which stays authoritative. A lookup becomes a hash lookup of one bucket plus a bounds check on the handful of candidates it returns. Ranges too wide to fan out go to a separate table scanned on every lookup, which is empty in practice and exists as a safety valve. 2^6 addresses per bucket, chosen by replaying all 213,969 production ranges against the 4,349,134 DevAddrs a node had actually seen: bits fan-out p99 max mean measured us/lookup 12 1.005x 2851 13553 77.90 229.4 10 1.020x 718 13538 20.04 8 1.081x 182 10749 5.26 6 1.323x 47 7932 1.38 6.26 The mean is what governs CPU, and 2^6 is 36.6x cheaper per lookup than 2^12 for 68k extra index rows. An on-node A/B against the live table measured 35.4ms -> 6.26us, with both paths returning identical route sets for every DevAddr sampled. There is no cache left to inspect or clear, so "config cache clear" and "config cache stats" become "config index rebuild" and "config index stats", and the hit/miss counters go away with them. * Store route ids as binaries hpr_route:id() is a string(), so a 36-character UUID costs 36 cons cells -- 576 bytes -- in every row that references it. The eui pair and devaddr range tables hold millions of such rows between them, and on a production node those char lists accounted for roughly half of the ETS the two tables used: 1.13GB of 2.16GB, against 56 bytes per id stored as a binary. Only the stored form changes. The module APIs still take and return strings, as do the protobufs, hpr_route_storage's keys, the CLI and the logs, so nothing outside these two modules can tell. foldl/2 converts back to the public form before handing a row to its callback: hpr_metrics:record_routes/0 and "config route refresh_broken" compare what it yields against hpr_route:id/1, and leaking the interned binary would silently make every comparison false and report every route with SKFs as broken. route_ids/0 is added for those two callers because they only want the distinct ids -- deduping in the stored form and converting the survivors avoids churning garbage per row on every tick. Both DETS files gain a version, with the previous file migrated on first boot and left in place so a rollback is just a redeploy. The route stream is incremental -- hpr_route_stream_worker persists last_timestamp -- so an emptied table would only be refilled by deltas, never by the full configuration; the tables have to be carried across the format change rather than resynced. * Take per-packet work off the routing path Three things were being computed for every packet purely to build lager metadata, on a path that also does the signature check and the route lookup. hpr_utils:gateway_name/1 is a base58check encode plus an md5 -- about 13us measured -- and the stream process was paying it per packet for a gateway it had already resolved at register time. It is now resolved once and passed through Opts. hpr_packet_up:md/2 reuses it whenever the packet's gateway matches the stream's, which is the case for every packet that survives hpr_routing:gateway_check/2, and falls back to computing it when they differ. The payload hash was computed twice: once by the service for last_phash and again by md/2 for its metadata. It is now hashed once and shared. The session key's base58 encode is about 10us and nothing reads the field except a human looking at a debug line, so it is only paid when something is listening -- debug enabled on the default sink, or any trace installed. The trace half matters: hpr_utils:trace/2 filters on the metadata built here, so a trace has to keep seeing complete metadata. The CT suite reached into #handler_state{} by position to read the session key, which silently shifts when a field is added ahead of it. It now goes through an accessor exported by the module that owns the record. * Reduce memory retained by the reporter and routing cache hpr_packet_reporter receives one cast per reported uplink while holding up to report_max_size of compressed report between uploads. With the mailbox on the process heap, every GC of that heap also walked the queued messages; off_heap stops that. hpr_routing_cache kept the packets queued behind a locked hash after the entry reached a terminal state. The entry lives for routing_cache_window_secs afterwards and is only ever read for its state and routes, so those retained packets did nothing but pin a payload for the rest of the window. Every terminal transition now clears them. vm.args stops the schedulers busy-waiting before they sleep. The latency floor here is the LoRaWAN Class A RX1 delay, a full second after the uplink, and the multi-buy grpc call alone is allowed 5s, so a futex wakeup in the tens of microseconds buys nothing measurable. The note about not capping the dirty schedulers is the important part of that file: msacc reports them almost entirely asleep, but it cannot see time spent inside a dirty NIF, and every packet's ed25519 signature check is one. Sizing them off msacc would be sizing them off a number that structurally cannot show the work. * Run every check in its own container, one per CT suite Adds docker-compose-test.yaml: a rustfs fixture plus one container per check, all from the Dockerfile's builder stage so the image is built once and shared. `make docker-test` brings it up, waits, replays each check's output and reports a single verdict. CT is split one container per suite. Suites are independent once each has its own network namespace, so the grpc servers CT starts on 8080/8085/8086/8087 and its data_dir no longer collide. The two reporter suites are the exception and share a container: both write test-bucket and both assert it is empty and empty it during cleanup, so running them concurrently against one rustfs would have them deleting each other's objects. The builder stage now carries test/ and a pre-built test profile, so the twelve CT containers start from a built tree instead of each repeating the same compile. test/ is copied after the release layer so editing a suite does not invalidate the dependency build above it. Replaces the old docker-test recipe, which ran `make test` inside the shipped image. That could not work: the runner stage carries only the release, with no source and no make. Also unbreaks the build itself. enacl hands ERL_NIF_INIT a function whose signature does not match the slot it lands in, and both gcc 14 -- which the Alpine image ships -- and clang 16+ turned that from a warning into an error, so neither the container nor a current macOS host could compile it. The Makefile now sets -Wno-error=incompatible-pointer-types for every build: spelled for the wider warning because gcc does not recognise clang's narrower name, and -Wno-error= rather than -Wno- so the diagnostic still shows up if it ever spreads to another NIF. h3 needs a CMake policy floor for the same reason on CMake 4 hosts. Drops templates/prometheus.yml and templates/promtail/config.yaml, left unreferenced when #341 removed promtail and prometheus from docker-compose.yaml. * Persist the gateway denylist across restarts A denied gateway was only denied until the next deploy: the table was in memory, so a restart silently went back to accepting the uplinks the list exists to drop. hpr_sup now creates and hydrates it from dets at startup, and changes are written through as they happen. Startup creation replaces create-on-first-add, which cannot work once persisted: is_denied/1 answers false for a missing table, so the list would stay dormant until someone added to it.
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.
Why
hpr_packet_reporterandhpr_gateway_liveness_reporterupload to different buckets, but both resolved credentials throughaws_credentials— a single VM-wide gen_server holding exactly one credential set, with no per-caller API. That works on AWS, where one instance role reaches both buckets, but cannot serve a deployment where each bucket has its own user/password. This unblocks moving the reporters off AWS.What changed
New
hpr_s3_client, shared by both reporters, resolving one of two targets:aws_endpointsetaws_access_key_id/aws_secret_access_key(required)aws_endpointunsetaws_bucket_regionaws_credentialsprovider chainClients are built with the
aws_clientconstructors rather than hand-rolled maps (aws_client()is opaque as of 1.3.2). One override remains:make_local_client/4is the only constructor accepting a custom endpoint and it pinsproto => http, so https endpoints patch that single field. A eunit test pins the local shape against the library constructor so a future aws-erlang bump fails there rather than silently in CT.Also bumps
awsto 1.3.2 andaws_credentialsto 1.1.1, and deletesconfig/sys.config, which no runtime path loaded and which had drifted out of sync.Tests moved from localstack to rustfs, matching oracles-iot.
docker-compose.yamlnow holds only rustfs plus an init container creatingtest-bucket;docker-compose-ct.yamlis gone.Bugs fixed along the way
token :=unconditionally; static credentials carry notokenkey. EC2 instance credentials do, so the current AWS deployment was unaffected — but this was directly in the path of the migration.try/catchintoobserve_*_report(error, ...).make test-awsset a host but no port, so every case silently skipped. Both are fixed, and the suites no longer skip themselves when the backend is missing — they fail.<<"Contents">>).Reviewer notes
aws_bucket_regiontoaws_region; that was reverted, so existing terraform-managed configuration works untouched and this branch can be deployed and rolled back on its own. A reporter with noaws_endpointbehaves exactly as today:aws_bucket_regionplus theaws_credentialschain.rustfsdoes not implement the batchDeleteObjectsAPI (aws_s3:delete_objects/3errors against it), so suite teardown deletes one key at a time. Reporters only callput_objectand are unaffected.docker-compose.yamlno longer defines thehpr,promtailandprometheusservices; it exists purely to back the test suites now.HPR_TEST_S3_ENDPOINTis kept for CI only: the job runs inside a container where rustfs resolves by service name, while compose publishes to localhost.ct.configcarries the local value.localinto the SigV4 credential scope regardless ofaws_region. rustfs accepts it; a stricter provider such as R2 may not — worth one realput_objectagainst the target bucket before pointing a reporter at it.Testing
fmt,xref,dialyzer,release, 200 eunit tests, and both reporter suites against rustfs viamake test-aws, all green locally.