feat(indexer): concurrency locks + aggregation index for indexer_metrics_collector, migration hooks + diagnostics for database_writer_pool - #369
Open
jonah-Elisha10 wants to merge 2 commits into
Conversation
…pool migration hooks + diagnostics Closes Goldii-locks#336: adds IndexerMetricsEventQueue, a bounded in-memory queue that serializes event inserts per identity (contractId|ledgerSequence|eventType, matching the events table's UNIQUE constraint). Concurrent notifications carrying the same event previously raced between the "already indexed?" check and the insert; the persisted-key set is now consulted inside the per-identity lock, so exactly one caller writes each row while unrelated events still persist concurrently. collectIndexerMetricsAsync adds single-flight de-duplication so concurrent collections drain the queue once and share one snapshot instead of racing several transactions over the same tables. Closes Goldii-locks#335: adds migration 6 creating idx_events_event_type. The collector's GROUP BY event_type aggregation previously planned as "SCAN events USING COVERING INDEX idx_events_ledger_event_type | USE TEMP B-TREE FOR GROUP BY"; it now plans as a covering index scan with no temp B-tree. Exports the exact query strings the collector runs plus EXPLAIN QUERY PLAN helpers, so the plan assertions verify the real queries rather than a copy that can drift. Candidate indexes for the webhook-subscription and active-contract counts were measured and rejected: those plans already use covering indexes, and a dead index only costs write throughput. Closes Goldii-locks#331: adds schema migration check utilities to database_writer_pool. verifyWriterPoolSchema reports missing migrations, tables, and columns; assertWriterPoolSchemaReady throws WriterPoolSchemaError; startWriterPool runs both plus any registered hooks and fails when the database state is out of sync. registerMigrationVerificationHook lets callers add their own checks; a hook that throws is reported as an issue rather than escaping the start. Start enforcement is opt-in via startWriterPool({ enforce: true }) so callers that never start the pool keep working exactly as before. Closes Goldii-locks#328: adds high-frequency debug diagnostics across the write path — enqueue, each write attempt, retries, failures, queue drains, and pool start. Every diagnostic message string carries elapsedMs=, plus payloadSizeBytes=, queueDepth= and attempt= where known, with the same values in structured meta. Also updates the migration-count assertion in indexer.test.ts, which pins the number of shipped migrations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@jonah-Elisha10 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! 🚀 |
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.
Implements four indexer issues across two modules.
Closes #336
Closes #335
Closes #331
Closes #328
#336 — Race conditions in concurrent
indexer_metrics_collectorcallsAdds
IndexerMetricsEventQueue, a bounded in-memory queue that serializes event inserts per identity (contractId|ledgerSequence|eventType, matching the events table's UNIQUE constraint).The race was the window between the "already indexed?" check and the insert: two concurrent notifications carrying the same event could both observe "not indexed" and both write. The persisted-key set is now consulted inside the per-identity lock, so exactly one caller writes each row while unrelated events still persist concurrently. Locks are released on throw,
enqueue/flush/submitare separable, andmaxQueueSizebounds memory.collectIndexerMetricsAsync()adds single-flight de-duplication on top: concurrent collections drain the queue once and share one snapshot instead of racing several transactions over the same tables.#335 — SQLite index structures for
indexer_metrics_collectorPlans were measured before choosing indexes. The events-by-type aggregation was the only query with a real problem:
Migration 6 adds
idx_events_event_type. Candidate indexes for the webhook-subscription and active-contract counts were measured and rejected — those plans already resolve through covering indexes, and a dead index only costs write throughput.The exact statements the collector runs are exported as
INDEXER_METRICS_QUERIES, so the query-plan assertions verify the real queries rather than a copy that can drift.INDEXER_METRICS_INDEXESnames every index the collector's lookups depend on (including the two from earlier migrations), andverifyIndexerMetricsIndexes()reports any that go missing.#331 — Migration verification hooks in
database_writer_poolverifyWriterPoolSchema()— reports missing migrations, tables, and columns without throwing, for callers that want to log or degrade.assertWriterPoolSchemaReady()— throwsWriterPoolSchemaErrorcarrying the issue list.startWriterPool()— runs both, plus any hooks registered throughregisterMigrationVerificationHook(), and fails when the database state is out of sync. A hook that throws is reported as an issue rather than escaping the start call.Start enforcement is opt-in via
startWriterPool({ enforce: true }). Making a successful start mandatory for writes would have broken every existing caller that usesqueueWritedirectly, so the default keeps current behaviour and enforcement is something a process opts into.#328 — Polling diagnostics logs for
database_writer_poolHigh-frequency debug diagnostics across the write path — enqueue, each write attempt, retries, failures, queue drains, and pool start. Every diagnostic message string carries
elapsedMs=, pluspayloadSizeBytes=,queueDepth=andattempt=where known, with the same values repeated in structured meta for log processors.Tests
Four new test files, 103 tests, all passing:
__tests__/indexer-metrics-collector-concurrency.test.ts__tests__/indexer-metrics-collector-indexes.test.tsidx_events_event_typeto prove it is load-bearing (the plan falls back to a temp B-tree without it)__tests__/database-writer-pool-migration-hooks.test.ts__tests__/database-writer-pool-diagnostics.test.tsOne existing assertion needed updating:
__tests__/indexer.test.tspins the number of shipped migrations, which migration 6 changes from 5 to 6.Full suite: 938 passed, 5 failed.
mainand untouched by this PR — verified by stashing these changes and re-running on a clean checkout, where they fail identically:__tests__/sqlite-schema-manager.test.ts(2) — references an unexportedSCHEMA_MANAGER_INDEXES, plus type errors__tests__/failover-recovery-backoff-retry.test.ts(2)__tests__/failover-recovery-poll-diagnostics.test.ts(1) — uses the barejestglobal, which is not injected under this repo's ESM Jest configThe same applies to the
npx tsc --noEmitstep in CI: 7 errors, all in test files this PR does not touch, all present onmain.tsc -p tsconfig.build.json(the production build) is clean. Happy to fix that pre-existing breakage in a follow-up if wanted.ℹ️ Note for merge ordering: PR #367 also modifies
src/indexer/indexer_metrics_collector.ts(alerting and diagnostics, issues #338/#337). The two touch different parts of the file, but whichever merges second will need a conflict resolution.🤖 Generated with Claude Code