when creating timeseries_data, never create unnecessary indexes#1644
Merged
fredclausen merged 61 commits intosdr-enthusiasts:mainfrom Mar 24, 2026
Merged
when creating timeseries_data, never create unnecessary indexes#1644fredclausen merged 61 commits intosdr-enthusiasts:mainfrom
fredclausen merged 61 commits intosdr-enthusiasts:mainfrom
Conversation
…ffer ## Summary Implements two complementary memory optimizations to reduce peak heap usage and eliminate expensive repeated work on client connect. ## Changes ### Backend — Tiered timeseries cache (utils/timeseries.ts, services/timeseries-cache.ts) - Added CacheTier type (warm | lazy | query-only) and tier field in PERIOD_CONFIG per period. - Exported WARM_PERIODS constant (1hr, 6hr, 12hr) derived from config. - Tier 1 (warm): 1hr, 6hr, 12hr — kept warm in RAM, refreshed on wall-clock schedule, pushed to all clients. - Tier 2 (lazy, TTL): 24hr, 1wk, 30day — query on first request, cached with TTL equal to refresh interval; not pushed unsolicited. - Tier 3 (query-only): 6mon, 1yr — always queried on demand; never stored in RAM on a timer. Eliminates the previously scheduled 1yr rebuild that caused ~420 MB periodic heap spikes. - getOrQueryTimeSeries(period) routes requests to the correct tier. ### Backend — Message ring buffer (services/message-ring-buffer.ts) - RingBuffer<T> implementation with fixed capacity and FIFO eviction. - Module-level push/get API: pushMessage, pushAlert (alert dedupe by uid), getRecentMessages, getRecentAlerts. - initMessageBuffers / warmMessageBuffers — seeds buffers at startup from DB (one read, no re-enrichment on connect). - resetMessageBuffersForTesting — test isolation helper. ### Backend — Wiring (services/index.ts, socket/handlers.ts, server.ts) - setupMessageQueue pushes enriched messages/alerts to ring buffers immediately after enrichment. - handleConnect reads from ring buffers instead of querying DB and re-enriching on every connect. - server.ts invokes initMessageBuffers + warmMessageBuffers at startup. ### Frontend (types/timeseries.ts, hooks/useSocketIO.ts, hooks/useRRDTimeSeriesData.ts) - Exported WARM_PERIODS from types. - useSocketIO: on connect emits rrd_timeseries only for WARM_PERIODS (3 requests instead of 8). - useRRDTimeSeriesData: for non-warm periods, emits rrd_timeseries on cache miss (lazy on-demand instead of eager on connect). ## Test coverage - Backend: 34 test files, 1033 tests passed (4 skipped) - Frontend: 36 test files, 1401 tests passed - New: message-ring-buffer unit + integration tests (891 lines) - Updated: timeseries-cache, handlers, integration, timeseries utils, useRRDTimeSeriesData, useSocketIO tests ## Memory impact - Removes periodic 1yr/6mon in-memory rebuild (was ~420 MB spike every 12 hr). - Connect path: zero DB queries, zero re-enrichment per connect. - Ring buffer overhead: bounded, small (defaults 250 msgs + 100 alerts).
…tches
Previously regenerateAllAlertMatches() called:
db.select().from(messages).all()
which materialises the entire messages table into a JS array at once.
On a large database (500k+ rows) this causes a significant transient
heap spike at exactly the moment when this already-expensive operation
is under load.
Two fixes applied together:
1. CURSOR-PAGINATED READS (WHERE id > lastId ORDER BY id LIMIT 1000)
Peak heap for the message array is now O(BATCH_SIZE) rather than
O(total messages). The INTEGER PRIMARY KEY rowid index makes each
page fetch O(log N) — no O(offset) cost on later pages as there
would be with LIMIT/OFFSET.
2. SINGLE better-sqlite3 TRANSACTION
The DELETE + resetAlertCounts + all INSERT/UPDATE writes are wrapped
in one conn.transaction() call. Benefits:
- Atomicity: a crash or error mid-run rolls back the DELETE so the
database is never left in a partially-cleared state.
- Throughput: bulk inserts in one transaction are ~100-1000x faster
than autocommit because each autocommit write is a separate
fsync-visible journal flush.
Additional cleanup:
- saveAlertMatch closure promoted out of the per-message loop (defined
once, takes message as a parameter).
- console.error replaced with logger.error.
Tests added:
- regression: processes all messages when count exceeds BATCH_SIZE (1000)
- regression: messages on a page boundary (exactly BATCH_SIZE) are all processed
- regression: non-matching messages on later pages are not counted as matched
- regression: entire regeneration is atomic (trigger-forced rollback
verifies the DELETE is rolled back when the transaction aborts)
beforeEach now also mocks getSqliteConnection (needed by conn.transaction()).
All 1037 backend tests pass.
Merge of upstream PR sdr-enthusiasts#1644 (wiedehopf). WHAT CHANGED ------------ Migration 9 created two indexes on timeseries_stats at table-creation time: CREATE INDEX idx_timeseries_timestamp_resolution ON timeseries_stats (timestamp, resolution); CREATE INDEX idx_timeseries_resolution ON timeseries_stats (resolution); Both indexes were always dead weight: - migration 11 dropped idx_timeseries_timestamp_resolution and recreated it as a UNIQUE index — work immediately undone by migration 12. - migration 12 dropped both indexes unconditionally and rebuilt the table with timestamp as INTEGER PRIMARY KEY (the rowid alias), making both indexes redundant anyway. Creating them in migration 9 only to discard them in 12 wastes time and disk space proportional to the number of timeseries rows — significant on a DB that has been running for months. CHANGES ------- - migration 9: remove both CREATE INDEX calls. - migration 11: remove the 'Step 2: Replace non-unique index with unique index' block — it created a UNIQUE index that migration 12 immediately dropped, achieving nothing. - migration 12: bare DROP INDEX → DROP INDEX IF EXISTS. Without this fix a fresh install (migration 9 never created the indexes) would hit migration 12 with no indexes to drop and throw. IF EXISTS makes the drop a safe no-op when the index is absent. - drizzle/0001_add_timeseries_stats.sql: remove matching CREATE INDEX lines (reference artifact, not executed at runtime). - dev-docs/TIMESERIES_STRATEGY.md: remove index SQL from schema example. REGRESSION TEST ADDED --------------------- 'regression: migration 12 DROP INDEX IF EXISTS is safe when indexes were never created (fresh install path)' — seeds a migration-11 state database WITHOUT the two indexes (the state produced by the updated migration 9) and asserts that runMigrations() does not throw and produces the correct final schema.
…nto memory-reduction
… into memory-reduction
also prevent them from being created during earlier migrations
wiedehopf
force-pushed
the
main
branch
6 times, most recently
from
09963bf to
f262d8b
Compare
tests yet to be updated
allow lower min height for charts
warn if icaos are encountered in unexpected formats
for searches on these columns, the FTS search is used thus a separate index for the column is not necessary
the main code should handle process.exit
…necessary_indexes2)
…hort viewports The CSS media query hiding .page__header below 800px viewport height causes h1 heading assertions to fail in Playwright's default 720px viewport. Replace with always-visible content-area selectors.
…visible Tests checking .page__stats and Mark All Read button need the page header which is hidden at viewport heights below 800px. Set viewport height to 900px at the start of those 3 tests.
…ct log levels The Dockerfile default was MIN_LOG_LEVEL=3 (warn), which suppressed all info-level output. Commit b286f1c worked around this by promoting info messages to warn. Fix the root cause by setting the default to 4 (info) and reverting the log level changes back to their correct severity.
The scale config had the numeric formatter and 'Count' title on x (the category axis) instead of y (the value axis). This caused Chart.js to display index numbers (0, 1, 2) instead of the label strings (Good Messages, Errors, Total). Swap the scale configs and fix tooltip to read parsed.y for vertical bars.
…age__stats Same root cause as the regular e2e fixes: .page__header is hidden at viewport heights below 800px, and Desktop Chrome defaults to 720px. Also removes deprecated baseUrl from tsconfig.app.json — TypeScript 6 resolves paths relative to the tsconfig directory by default.
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
2 participants
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.
save space and time during migration process