feat(sqlite_schema_manager): add poll speed and payload size diagnostics logging - #372
Merged
2 commits merged intoAug 30, 2026
Conversation
|
@solaawojobi00-bit 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! 🚀 |
Log elapsed time and payload size for insertEventBatch, the hot write path invoked on every indexer poll cycle, mirroring the poll diagnostics convention already used in failover-recovery.ts. Also fixes two pre-existing bugs in this test file uncovered while adding coverage: an undefined SCHEMA_MANAGER_INDEXES reference and a stale migration-count assertion that predated the version-4/5 migrations. Closes Goldii-locks#261
Unrelated to Goldii-locks#261 but blocking a clean CI run on main: - failover-recovery-poll-diagnostics.test.ts and failover-recovery-backoff-retry.test.ts were missing `import { jest } from "@jest/globals"`, causing `ReferenceError: jest is not defined` under ESM. - indexer-runner-historical-sync.test.ts typed mockGetEvents as a zero-arg mock but every real call site passes an options object. - jest.spyOn(logger, ...) call sites hit a TS tuple-typing quirk from winston's overloaded LeveledLogMethod signature; cast the mock.calls tuples explicitly instead. failover-recovery-backoff-retry.test.ts still has a separate, pre-existing bug (fake timers vs. retryWithBackoff's real setTimeout causing a test timeout) left unfixed here as it requires debugging failover-recovery.ts's retry logic, which is out of scope for this PR.
solaawojobi00-bit
force-pushed
the
fix/issue-261-schema-manager-poll-diagnostics
branch
from
August 30, 2026 03:00
6d389fc to
ca766f9
Compare
9e42e6f
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.
Problem
sqlite_schema_manager(src/indexer/db.ts) had no debug-level diagnostics tracking how long its writes take or how large the payloads it processes are, making it hard to spot slow polling cycles or unusually large event batches in production logs.insertEventBatchelapsedMs=<n>Solution
Added
logSchemaManagerPollDiagnostics(operation, startedAtMs, payloadSizeBytes)tosqlite_schema_manager, mirroring the diagnostics convention already established bylogPollDiagnosticsinfailover-recovery.ts: a debug log whose message string embedselapsedMs=/payloadSizeBytes=plus structured metadata for log aggregators. Wired it intoinsertEventBatch, the write path invoked on every indexer poll cycle when new events are found, so poll speed and payload size are captured on the module's actual hot path.Changes
src/indexer/db.tslogSchemaManagerPollDiagnostics()— computeselapsedMsfrom a caller-suppliedstartedAtMsand logs a debug diagnostic string + metadata object.insertEventBatch()now records a start timestamp and calls the new diagnostics function after the transaction commits, reporting the serialized batch's byte size aspayloadSizeBytes.SCHEMA_MANAGER_INDEXES(fixes a pre-existing undefined reference in the test suite — see Notes for Reviewers).__tests__/sqlite-schema-manager.test.tsinsertEventBatchintegration.[1,2,3], which predated the already-merged version 4/5 migrations.Regression Tests
logSchemaManagerPollDiagnosticstest asserts the message containselapsedMs=and the metadata'selapsedMsis a real, non-negative numberpayloadSizeBytes=2048in the message and metadatainsertEventBatch diagnosticstest inserts a real batch and asserts the emitted debug log carriesoperation=insertEventBatch,elapsedMs=, and apayloadSizeBytesgreater than zerotsc --noEmitrun clean (see Testing)Testing
Notes for Reviewers
While adding test coverage for this issue I hit three separate pre-existing bugs unrelated to
sqlite_schema_managerthat were breakingtsc --noEmitand/ornpm testonmainalready (verified by stashing my changes and re-running against a clean checkout). Since CI runs both steps, I fixed the ones that were mechanical:__tests__/failover-recovery-poll-diagnostics.test.tsand__tests__/failover-recovery-backoff-retry.test.tswere missingimport { jest } from "@jest/globals", causingReferenceError: jest is not definedunder this project's ESM Jest config.__tests__/sqlite-schema-manager.test.tsreferenced an undefinedSCHEMA_MANAGER_INDEXES— now exported fromdb.ts.__tests__/indexer-runner-historical-sync.test.tstyped itsmockGetEventsmock as taking zero arguments, but the real call site passes an options object.jest.spyOn(logger, ...)call sites hit a TypeScript tuple-typing quirk from winston's overloadedLeveledLogMethodsignature (.mock.calls[0]resolving to a 1-element tuple); fixed with explicit casts, consistent with the style used in the new tests added here.One remaining known issue, left untouched:
failover-recovery-backoff-retry.test.tshas a deeper, pre-existing bug — fake timers combined withretryWithBackoff's realsetTimeoutcalls causes a 5s test timeout, plus some untypedjest.fn()mocks. This is unrelated tosqlite_schema_managerand needs dedicated debugging offailover-recovery.ts's retry logic, so I left it for a separate PR. It accounts for the 1 failed suite / 2 failed tests shown above.Closes #261