Fix indexer startup failure with large dynamic contract sets - #1259
Conversation
getInitialState aggregates the entire envio_addresses table into one json column via json_agg. With enough dynamic contracts the aggregated value exceeds V8's max string length and postgres.js throws ERR_STRING_TOO_LONG while decoding the row, so the indexer cannot resume. The test seeds a chain with dynamic-contract rows whose combined contract_name length exceeds the limit and asserts getInitialState returns them all. It currently fails with the exact ERR_STRING_TOO_LONG error from the issue, demonstrating the bug. https://claude.ai/code/session_01BXuRQX5sq8KKfqoKcGRLZv
getInitialState aggregated the entire envio_addresses table per chain with json_agg, producing a single column value that postgres.js decodes via Buffer.toString. Past V8's max string length (0x1fffffe8) that throws ERR_STRING_TOO_LONG and the indexer cannot resume. Read the addresses as plain rows in a separate query and group them by chain in JS, so no individual column value can overflow. https://claude.ai/code/session_01BXuRQX5sq8KKfqoKcGRLZv
|
Warning Review limit reached
More reviews will be available in 19 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactored dynamic contract state loading to split indexing addresses into a separate query executed in parallel with the initial chain state query. This avoids exceeding V8's string-length limits when large contract names aggregate into JSON. Added async merging logic and comprehensive test coverage including a regression test for oversized contract payloads. ChangesIndexing-addresses refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
The repro pushes ~600MB through Postgres to cross the V8 string limit, too slow for every CI run. Keep it as a manually-runnable guard. https://claude.ai/code/session_01BXuRQX5sq8KKfqoKcGRLZv
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/envio/src/db/InternalTable.res (1)
215-249: ⚡ Quick winSplit the raw DB row type from the merged return type.
makeGetInitialStateQueryno longer returnsindexingAddresses, but the cast here still treats those rows asrawInitialState. That makes the unsafe cast claim a field exists before the JS merge adds it. A dedicated raw-chain-row type would keep the cast aligned with the SQL and avoid future undefined-field bugs.🤖 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 `@packages/envio/src/db/InternalTable.res` around lines 215 - 249, The code incorrectly casts DB rows from makeGetInitialStateQuery as containing indexingAddresses; update getInitialState to use a distinct raw row type for rows returned by makeGetInitialStateQuery (e.g., rawInitialStateRow) and a separate rawIndexingAddress type for makeGetIndexingAddressesQuery, then only merge indexingAddresses into the final mapped object after building indexingAddressesByChainId; specifically change the unsafe cast calls around Postgres.unsafe(makeGetInitialStateQuery(~pgSchema)) and Postgres.unsafe(makeGetIndexingAddressesQuery(~pgSchema)) to cast to the appropriate raw types, and in the rawInitialStates->Array.map step construct the final shape by adding indexingAddresses from indexingAddressesByChainId (using rawInitialState.id->Int.toString) rather than assuming the field exists on the DB row.
🤖 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 `@packages/envio/src/db/InternalTable.res`:
- Around line 215-223: getInitialState currently issues two independent
Postgres.unsafe calls which can observe different snapshots; wrap both reads in
a single repeatable-read transaction so they see the same DB snapshot. Modify
getInitialState to begin a transaction with isolation level REPEATABLE READ (or
use the library's transaction helper, e.g. Postgres.transaction or
Postgres.withTransaction) and inside that transaction invoke
makeGetInitialStateQuery and makeGetIndexingAddressesQuery on the same
transaction-bound `sql` object (replacing the current Promise.all2 flow), then
cast results to rawInitialState/rawIndexingAddress and return before committing;
ensure you use the same connection/`sql` for both Postgres.unsafe calls so both
reads use the same snapshot.
---
Nitpick comments:
In `@packages/envio/src/db/InternalTable.res`:
- Around line 215-249: The code incorrectly casts DB rows from
makeGetInitialStateQuery as containing indexingAddresses; update getInitialState
to use a distinct raw row type for rows returned by makeGetInitialStateQuery
(e.g., rawInitialStateRow) and a separate rawIndexingAddress type for
makeGetIndexingAddressesQuery, then only merge indexingAddresses into the final
mapped object after building indexingAddressesByChainId; specifically change the
unsafe cast calls around Postgres.unsafe(makeGetInitialStateQuery(~pgSchema))
and Postgres.unsafe(makeGetIndexingAddressesQuery(~pgSchema)) to cast to the
appropriate raw types, and in the rawInitialStates->Array.map step construct the
final shape by adding indexingAddresses from indexingAddressesByChainId (using
rawInitialState.id->Int.toString) rather than assuming the field exists on the
DB row.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 078979ea-04ae-4903-8cec-9f4162057727
📒 Files selected for processing (3)
packages/envio/src/db/InternalTable.resscenarios/test_codegen/test/lib_tests/DynamicContractsStartupSize_test.resscenarios/test_codegen/test/lib_tests/PgStorage_test.res
| let getInitialState = async (sql, ~pgSchema) => { | ||
| let (rawInitialStates, rawIndexingAddresses) = await Promise.all2(( | ||
| sql | ||
| ->Postgres.unsafe(makeGetInitialStateQuery(~pgSchema)) | ||
| ->(Utils.magic: promise<array<unknown>> => promise<array<rawInitialState>>), | ||
| sql | ||
| ->Postgres.unsafe(makeGetIndexingAddressesQuery(~pgSchema)) | ||
| ->(Utils.magic: promise<array<unknown>> => promise<array<rawIndexingAddress>>), | ||
| )) |
There was a problem hiding this comment.
Keep both startup reads on one database snapshot.
The old implementation got chain state and indexing addresses from one SQL statement; this split now reads them from two independent statements. Under Postgres' default READ COMMITTED behavior, a contract inserted or removed between these calls can make getInitialState resume from mismatched chain and address state. Please run both reads under a repeatable-read transaction, or otherwise force a single snapshot.
🤖 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 `@packages/envio/src/db/InternalTable.res` around lines 215 - 223,
getInitialState currently issues two independent Postgres.unsafe calls which can
observe different snapshots; wrap both reads in a single repeatable-read
transaction so they see the same DB snapshot. Modify getInitialState to begin a
transaction with isolation level REPEATABLE READ (or use the library's
transaction helper, e.g. Postgres.transaction or Postgres.withTransaction) and
inside that transaction invoke makeGetInitialStateQuery and
makeGetIndexingAddressesQuery on the same transaction-bound `sql` object
(replacing the current Promise.all2 flow), then cast results to
rawInitialState/rawIndexingAddress and return before committing; ensure you use
the same connection/`sql` for both Postgres.unsafe calls so both reads use the
same snapshot.
The test coordinates source recovery with real timers around a 50ms recovery timeout, which is occasionally too tight under CI load. Retry up to 3 times. https://claude.ai/code/session_01BXuRQX5sq8KKfqoKcGRLZv
2 participants
Summary
Fixes a critical issue where indexers with many registered dynamic contracts would fail to start due to exceeding V8's maximum string length when aggregating contract data in a single JSON column.
Problem
On startup,
InternalTable.Chains.getInitialStateloads all registered dynamic contracts for each chain by aggregating the entireenvio_addressestable into a single JSON column usingjson_agg. With enough contracts (approximately 120+ contracts with typical metadata), the aggregated JSON string exceeds V8's maximum string length (0x1fffffe8), causingpostgres.jsto throwERR_STRING_TOO_LONGduring row decoding. This made it impossible for indexers to resume.Solution
Split the data loading into two separate queries:
makeGetInitialStateQuery: Loads chain state without the addresses aggregatemakeGetIndexingAddressesQuery: Loads all addresses as individual rowsThe
getInitialStatefunction now:Promise.all2This approach avoids creating oversized JSON strings while maintaining the same API contract.
Changes
InternalTable.Chains.makeGetInitialStateQueryto remove thejson_aggsubqueryInternalTable.Chains.makeGetIndexingAddressesQueryto fetch addresses as individual rowsInternalTable.Chains.getInitialStateto be async and handle grouping in JavaScriptDynamicContractsStartupSize_test.resto verify the fix handles 120+ contracts with 5MB metadata eachPgStorage_test.reshttps://claude.ai/code/session_01BXuRQX5sq8KKfqoKcGRLZv
Summary by CodeRabbit
Bug Fixes
Performance