fix(compose): forward the observer's cranker and cleanup settings - #857
Merged
Conversation
The observer reads ten settings that docker-compose never passed into its container, so setting any of them in .env silently did nothing: CRANK_BATCH_SIZE CLEANUP_BATCH_SIZE CRANK_CLOSE_EPOCHS CLEANUP_FAILURE_THRESHOLD CRANK_EPOCH_RETENTION MAX_CLEANUP_TXS_PER_CYCLE CRANK_WARN_BALANCE_SOL ALT_RECLAIM_SCAN_LIMIT CRANK_CRITICAL_BALANCE_SOL OBSERVED_GATEWAY_HOSTS This is the same gap #842 fixed for CRANK_POLL_INTERVAL_MS / CLEANUP_MIN_INTERVAL_MS and #846 fixed for CLEANUP_TO_RETURNED_TXS_PER_CYCLE; the rest of the family was never wired. CRANK_EPOCH_RETENTION is the one that prompted this. It governs how many epochs of on-chain history survive before close_epoch reclaims their rent, and closing an epoch also closes its Observation PDAs - which is what removes per-observer report detail from chain. On mainnet only the two most recent epochs still had Observation accounts, so an operator wanting more history had no way to ask for it. ALT_RECLAIM_SCAN_LIMIT matters for the same reason: reclaiming a prescribe Address Lookup Table removes the observer index for its epoch, so getEpoch can no longer enumerate that epoch's reports. Verified against the running observer image (15e285b0) that an empty value is a true no-op rather than a parse error: env.varOrDefault returns the default for both empty and whitespace-only values, and loading dist/config.js with all ten set to '' yields every documented default unchanged (retention 7, batch 15, close true, scan limit 200, cleanup txs 50, thresholds 30/0.3/0.1). That mattered because parsePositiveIntEnv throws on a non-positive integer, so a blank reaching it would have failed observer startup for every deployment. Documented in docs/envs.md per the compose/envs.md sync rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe observer service now receives cranking, cleanup, balance-alert, ALT-reclamation, and gateway-host settings from environment variables. Documentation and the changelog describe these settings and empty-value default behavior. ChangesObserver configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🔵 Low · up to The change makes observer cranker and cleanup settings configurable through Compose, including retention and host-list settings. The PR is mergeable with owner awareness because whitespace in the comma-separated observed-host setting is not clearly defined and could cause a configured host to be skipped. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/envs.md`:
- Line 189: Update the OBSERVED_GATEWAY_HOSTS documentation and observer
handling to define whitespace behavior: either require and document the no-space
format host-a,host-b, or trim each parsed host entry. Add a regression check
covering the input host-a, host-b and ensure both hosts are recognized.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 364a246c-2d12-4812-a0c4-6d92c05f5054
📒 Files selected for processing (3)
CHANGELOG.mddocker-compose.yamldocs/envs.md
| | CLEANUP_FAILURE_THRESHOLD | Number | 30 | Consecutive-failure count at which a gateway becomes eligible for `prune_gateway`. Mirrors the on-chain `EpochSettings.max_consecutive_failures`. | | ||
| | MAX_CLEANUP_TXS_PER_CYCLE | Number | 50 | Hard cap on cleanup transactions submitted per cleanup cycle, shared across all cleanup phases. Bounds gas spend if discovery returns thousands of stale accounts. | | ||
| | ALT_RECLAIM_SCAN_LIMIT | Number | 200 | Recent signatures scanned per pass when reclaiming rent from single-use prescribe Address Lookup Tables (~0.0126 SOL each). Set `0` to disable reclamation. Reclaiming a prescribe ALT removes the observer index for its epoch, so `getEpoch` can no longer enumerate that epoch's reports even while the underlying accounts live. | | ||
| | OBSERVED_GATEWAY_HOSTS | String | (all registered gateways) | Comma-separated list restricting which gateway hosts this observer assesses. Empty observes every gateway in the registry. | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- documentation context ---'
sed -n '175,192p' docs/envs.md
printf '%s\n' '--- observer parser ---'
curl -fsSL \
https://raw.githubusercontent.com/ar-io/ar-io-observer/15e285b0a539dfb732ab70e13ab062c02c9906d0/src/config.ts \
| nl -ba | sed -n '1,220p'Repository: ar-io/ar-io-node
Length of output: 6493
🏁 Script executed:
#!/bin/bash
set -eu
curl -fsSL \
https://raw.githubusercontent.com/ar-io/ar-io-observer/15e285b0a539dfb732ab70e13ab062c02c9906d0/src/config.ts \
| sed -n '1,240p'Repository: ar-io/ar-io-node
Length of output: 7739
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- OBSERVED_GATEWAY_HOSTS references ---'
rg -n -C 4 'OBSERVED_GATEWAY_HOSTS|observedGatewayHosts|observed-gateway-hosts' . \
-g '!node_modules' -g '!dist' -g '!build'Repository: ar-io/ar-io-node
Length of output: 4748
🏁 Script executed:
node - <<'JS'
const values = ['host-a,host-b', 'host-a, host-b', ''];
for (const value of values) {
const parsed = value.split(',').filter((host) => host.length > 0);
console.log(JSON.stringify({ value, parsed }));
}
JSRepository: ar-io/ar-io-node
Length of output: 291
Define whitespace handling for OBSERVED_GATEWAY_HOSTS.
The observer does not trim entries. Document a no-space format, such as host-a,host-b, or trim entries in the observer. Add a regression check for host-a, host-b.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/envs.md` at line 189, Update the OBSERVED_GATEWAY_HOSTS documentation
and observer handling to define whitespace behavior: either require and document
the no-space format host-a,host-b, or trim each parsed host entry. Add a
regression check covering the input host-a, host-b and ensure both hosts are
recognized.
Source: MCP tools
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #857 +/- ##
========================================
Coverage 79.78% 79.79%
========================================
Files 141 141
Lines 54342 54342
Branches 4171 4169 -2
========================================
+ Hits 43359 43364 +5
+ Misses 10931 10925 -6
- Partials 52 53 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The observer splits this var on ',' without trimming, so 'a.com, b.com' yields a second entry of ' b.com' that matches no gateway and silently narrows the observation set. Verified against the 15e285b0 image: 'host-a, host-b' -> ["host-a"," host-b"] 'host-a,host-b' -> ["host-a","host-b"] Documenting the constraint here since the parsing lives in ar-io-observer. A follow-up there adds the trim (REFERENCE_GATEWAY_HOSTS, defined directly above it, already does), after which this note can be dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Handled the CodeRabbit finding — it was correct. Verified against the Worth noting it fails silently: the malformed entry just never matches a gateway, so the observation set narrows with no error. Addressed both halves of the suggestion rather than picking one:
Once #123 ships, the doc note here can be dropped. Also flagging why this surfaced now: |
pull Bot
pushed a commit
to AKJUS/ar-io-observer
that referenced
this pull request
Aug 13, 2026
OBSERVED_GATEWAY_HOSTS and ARNS_NAMES split on ',' without trimming, so the natural way to write a list breaks them: 'a.com, b.com' -> ['a.com', ' b.com'] The ' b.com' entry matches no gateway, so it fails SILENTLY - the host is simply never found and the observation set narrows, with no error to explain why. ARNS_NAMES has the same shape. REFERENCE_GATEWAY_HOSTS, defined directly above these two, already does .map((h) => h.trim()). This aligns the other two with that behavior rather than introducing a new convention. Extracted parseCommaSeparatedList so the parsing is unit-testable: config.ts runs its parsing at module load, so the values themselves can only be exercised by reimporting the module under mutated process.env. Behavior is unchanged for well-formed input: already-trimmed lists parse identically, and an unset var still yields [] (config passes '' when neither env nor CLI arg is set, and an empty list means 'no restriction'). Verified against the built dist: 'host-a, host-b' -> ["host-a","host-b"] unset -> [] Found by CodeRabbit on ar-io/ar-io-node#857, which forwards OBSERVED_GATEWAY_HOSTS into the observer container for the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The observer reads ten settings that
docker-compose.yamlnever passed into its container, so setting any of them in.envsilently did nothing:Same gap #842 fixed for
CRANK_POLL_INTERVAL_MS/CLEANUP_MIN_INTERVAL_MSand #846 fixed forCLEANUP_TO_RETURNED_TXS_PER_CYCLE— the rest of the family was never wired.What prompted it
CRANK_EPOCH_RETENTIONgoverns how many epochs survive beforeclose_epochreclaims their rent, and closing an epoch also closes its Observation PDAs — which is what removes per-observer report detail from chain.Measured on mainnet: only the two most recent epochs still had Observation accounts (13 for epoch 510, 9 for 511, nothing older), while the Epoch PDAs for 506–509 survived holding only aggregate
observationsSubmittedcounts. An operator wanting more history had no way to ask for it.ALT_RECLAIM_SCAN_LIMITmatters for the same reason from the other direction: reclaiming a prescribe Address Lookup Table removes the observer index for its epoch, sogetEpochcan no longer enumerate that epoch's reports even while the underlying accounts are alive.Worth noting for anyone reading this later:
close_observationandclose_epochare permissionless, so the effective retention across the network is set by the most aggressive cranker running, not by any single operator. This PR makes the setting reachable; it doesn't make it authoritative.Regression check
This one had a real failure mode worth verifying rather than assuming.
parsePositiveIntEnvdoesNumber.parseInt(raw)and throws on a non-positive integer — so if${VAR:-}rendered an empty string that reached it, the observer would fail to start on every deployment that doesn't set these.Verified against the running observer image (
15e285b0):env.varOrDefaultreturns the supplied default for both empty and whitespace-only values, so a blank never reaches the numeric parsers.dist/config.jswith all ten set to''yields every documented default unchanged:Also confirmed
docker compose configrenders the values through when set (CRANK_EPOCH_RETENTION: "14",ALT_RECLAIM_SCAN_LIMIT: "0") and empty when unset.Scope
Seven observer settings remain unforwarded —
PORT(intentional; compose mapsOBSERVER_PORT),ARNS_NAMES,ALWAYS_SAVE_REPORTS,REFERENCE_GATEWAY_HOST,BLOCK_OFFSET_MAPPING_ENABLED,TX_PATH_PARSING_ENABLED,ENABLE_OPENAPI_VALIDATION. Those are observer-internal/dev knobs rather than operational cranker settings, so I left them out to keep this coherent. Happy to add them if preferred.Documented in
docs/envs.mdper the compose/envs.md sync rule.