feat(indexer): test SQLite + Postgres backend matrix and document selection - #452
Open
Topmatrixmor2014 wants to merge 3 commits into
Open
feat(indexer): test SQLite + Postgres backend matrix and document selection#452Topmatrixmor2014 wants to merge 3 commits into
Topmatrixmor2014 wants to merge 3 commits into
Conversation
) The indexer supports both SQLite and Postgres via DB_DRIVER, but the DB layer was never exercised on any backend by CI — the whole package had no workflow job and, as it turns out, did not even compile. Add a parameterized DB test matrix that runs the SAME suite (migrations, cursor updates, upserts, revokes, claimsByWallet, stats, recent, deleteClaimsAfter, getMaxClaimLedger) against SQLite and a real Postgres, plus CI wiring and backend-selection documentation. Fixes surfaced by the matrix: - db.ts: pg returns BIGINT/INTEGER as strings; register numeric type parsers so Postgres rows match SQLite's number shapes. - ingester.ts: repair a corrupted fetchEvents (redeclared vars, undefined lastLedger) from a bad merge, and move reorg detection ahead of the finality-ceiling early return so a reorged cursor is actually reconciled. - ingester.ts: decodeScVal now falls back to the literal string for non-XDR values so plain-string event topics parse correctly. - api.test/ingester.test: fix pre-existing Config/stub mismatches (missing reconcile in the Ingester stub, newer Config fields). CI: add an `indexer` job (SQLite + Postgres service container) that builds and runs the full indexer suite. Docs: add a "Database Backend Selection & Tradeoffs" section covering DB_DRIVER, when to pick each engine, and how to exercise both backends in the test matrix. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@Topmatrixmor2014 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Collaborator
|
Good work on the dual-backend test matrix. It now conflicts with main though: #448 (keyset cursor pagination) just merged and also edits services/indexer/src/db.ts, ingester.ts, and api.ts. Please rebase onto latest main. The pagination change altered the db query shape, so make sure the Postgres and SQLite matrix tests cover the new keyset queries after the rebase. |
PR ToluLabs#448 switched recent() to keyset (cursor) pagination returning a RecentPage, but db.test.ts still called the old offset-based API, so the SQLite + Postgres matrix failed to build/run. Update the pagination test to the new { claims, nextCursor } shape and add a fourth claim so a second page genuinely exists — exercising the keyset cursor and proving revoked claims stay excluded across pages. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.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
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.
Summary
Closes #350
The indexer (
services/indexer) advertises two storage backends — SQLite viabetter-sqlite3and Postgres viapg— selected byDB_DRIVER. But the DB layer was never exercised by CI, and (as this work surfaced) the whole package did not even compile or pass its own tests. Only one backend being run means the other silently rots, and the two use different SQL dialects (INSERT OR IGNOREvsON CONFLICT,INTEGERvsBIGINT) exactly where they diverge.This PR:
BIGINT-as-string rows, a corruptedfetchEvents, reorg detection that could never fire, and non-compiling tests).What changed
New parameterized DB test matrix —
services/indexer/src/db.test.tsA single test suite registered for both
sqliteandpostgres, covering everything issue #350 calls out:getLastLedger/setLastLedgerround-trip, default 0)revokedreset on re-verify)revoked = 1, and cleared again on re-verification)claimsByWallet,stats(total/active/revoked), paginatedrecentdeleteClaimsAfterrollback andgetMaxClaimLedgerThe Postgres leg reads
TEST_POSTGRES_URL(falling back toDATABASE_URL): it runs in CI and locally when a Postgres is reachable, and skips gracefully when it isn't (so localnpm testnever fails for lack of a DB).CI —
.github/workflows/ci.ymlAdds an
indexerjob with apostgres:16service container (POSTGRES_USER/PASSWORD/DB=indexer) that:npm ci) and builds (npm run build),TEST_POSTGRES_URLpointing at the container — sodb.test.tsruns both the SQLite and Postgres legs on every PR. This is the first time the indexer has ever had CI coverage.Documentation
services/indexer/README.md— new "Database Backend Selection & Tradeoffs" section: how a deploy selects the backend (DB_DRIVER), whatDATABASE_URL/SQLITE_PATHcontrol, when to pick each engine (dev/single-instance vs production multi-instance), concurrency/writing constraints, operational tooling, and how to run the DB matrix locally.services/indexer/.env.example— documentsDB_DRIVER,SQLITE_PATH,DATABASE_URL, andTEST_POSTGRES_URL.Real bugs fixed along the way (surfaced by the matrix)
db.ts—node-postgresreturnsBIGINT/INTEGERcolumns as strings, while SQLite returns numbers, soclaimsByWallet/stats/recentexposed different row shapes per backend. Registered pg numeric type parsers (types.setTypeParser) so Postgres matches SQLite exactly.ingester.ts— repaired a corruptedfetchEvents(a bad merge left redeclaredpage/records, an undefinedlastLedger, and a duplicate implementation) and moved reorg detection ahead of the finality-ceiling early return so a reorged cursor (paused > head) is actually reconciled instead of silently treated as "ahead".ingester.ts—decodeScValnow falls back to the literal string when input isn't valid base64 XDR, so plain-string event topics ("proof","verified") parse correctly instead of collapsing tonull.api.test.ts/ingester.test.ts— fixed pre-existing mismatches: theIngesterstub was missing thereconcilemember, and test config factories were missing the newerConfigfields (corsOrigins,rateLimit*), which is why the package didn't compile. The reconcile test was also corrected to match the documented "deletes and re-indexes" behavior.Validation
services/indexer:tsc --noEmit— cleannpm run build— succeedsnpm test(SQLite leg) — 55 passed / 1 skipped (Postgres skipped without a URL)npm testwith a real Postgres 16 (docker run -p 5432:5432 postgres:16) — 66 passed (SQLite + Postgres legs both green).github/workflows/ci.yml— parsed and validated (jobs:commitlint,contracts,circuits,frontend,indexer,sdk-integration,a11y)Notes
fetchEvents/reorg fixes are changes to production behavior, but they only make the two backends behave identically and restore the finality/reorg design intent — they were unreachable/broken before.ingester.test.tsreflect the intended, documented behavior rather than weakening any previously-passing assertions.closes #350