Skip to content

fix: partition messages_lookup to stop the index bloat - #4106

Merged
darshankabariya merged 7 commits into
masterfrom
fix_slowquery
Aug 19, 2026
Merged

fix: partition messages_lookup to stop the index bloat #4106
darshankabariya merged 7 commits into
masterfrom
fix_slowquery

Conversation

@darshankabariya

@darshankabariya darshankabariya commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Store nodes have two tables. The big one (messages) is cleaned by dropping hourly partitions — instant and clean. The small helper table (messages_lookup) is cleaned by deleting rows every 30 minutes. In Postgres, deleted rows leave dead space in indexes that is never given back. After months of this, ~20 MB of real data was buried under up to 1.3 GB of dead index per node — and while each cleanup pass waded through that dead space, the whole database briefly stalled, so any query running at that moment took >1s, even SELECT version();. That is the slowQuery alert storm in #3790.

Fix

Make messages_lookup partitioned, exactly like messages:

  • a lookup partition is created together with each hourly messages partition,
  • retention drops them together — no row deletes in time/size retention, so the dead-space buildup becomes impossible,
  • every factory pass reconciles the two tables: missing lookup partitions are rebuilt from stored messages (crash-safe — a partition only becomes visible after its backfill completed), which also auto-repairs databases where the table was dropped by hand.

Tests: three new ones (partition lockstep; rebuild path; stray cleanup), plus the full archive suite — 155/155 against real Postgres 15.

Upgrade note

Right after the migration, hash-based store queries may briefly return empty while messages_lookup is rebuilt from stored messages (newest first; seconds on current fleet data).

What this PR does NOT fix (known, tracked separately)

  • capacity:N retention still row-deletes — an exact row-count promise cannot be expressed as partition drops. Documented in code; unused by our fleets; convert-or-deprecate tracked in chore(archive): deprecate capacity retention policy #4123.
  • Client query storms: a single js-waku client sending ~800 concurrent store queries each morning saturates the DB regardless of schema. Needs store request rate-limiting enabled in the fleet config (the code support already exists) — follow-up issue.

close #3790

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

This PR may contain changes to database schema of one of the drivers.

If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues.

Please make sure the label release-notes is added to make sure upgrade instructions properly highlight this change.

@darshankabariya
darshankabariya marked this pull request as draft August 4, 2026 22:42
Comment thread migrations/message_store_postgres/pg_migration_manager.nim
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

You can find the images built from this PR at

quay.io/wakuorg/nwaku-pr:4106
quay.io/wakuorg/nwaku-pr:4106-logosdeliverynode

Built from 7a09731

@darshankabariya darshankabariya changed the title fix: partition messages_lookup so retention drops partitions instead … fix: partition messages_lookup to stop the index bloat Aug 10, 2026
@darshankabariya
darshankabariya marked this pull request as ready for review August 10, 2026 10:19
darshankabariya and others added 2 commits August 12, 2026 15:34
Review round on #4106:
- create+backfill now run atomically inside the advisory-lock DO block,
  so an interruption cannot leave a permanently empty lookup partition
- reconciliation runs every factory iteration (newest-first) instead of
  once pre-loop: it no longer gates the 10s startup partition wait, the
  current hour heals first, and gaps opened by old-binary writers on a
  shared database self-repair within one iteration
- stray lookup partitions with no messages sibling are dropped, closing
  the boot fatal-loop from range overlaps
- notes: lock trade-off comments, v8 comment covers the timestamp index,
  getPartitionsList parameterized, test db url deduplicated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment still refers to deleting rows

@stubbsta

Copy link
Copy Markdown
Contributor

Some possible risks identified by Claude:

  1. Medium — Capacity retention still row-deletes from messages_lookup
    postgres_driver.nim:1047-1056— deleteOldestMessagesNotWithinLimit (the Capacity retention policy) still runs:

DELETE FROM messages_lookup WHERE messageHash NOT IN (SELECT messageHash FROM messages ORDER BY timestamp DESC LIMIT ?)
This is exactly the row-delete pattern the PR sets out to eliminate — the PR body says "no row deletes anywhere," but this path reintroduces the same dead-index bloat for any node using capacity-based retention. The time/size-based path is fixed; this one isn't. Either convert this to drop-partition semantics too, or explicitly scope the PR/issue to non-capacity retention and note the gap. At minimum it contradicts the stated invariant.

  1. Medium — Silent degraded window after migration / heal
    Hash-based store queries (getMessagesByMessageHashes, postgres_driver.nim:811-842) rely entirely on messages_lookup. Right after the v8 migration the table is empty and stays empty until ensureLookupPartitions finishes backfilling every historical partition — and the query returns an empty result, not an error (your own "degraded query" test asserts len == 0). On a node with months of history this backfill can be lengthy, and during it hash lookups silently return "not found" rather than surfacing a transient failure. Worth: (a) confirming the backfill runs early/fast enough on real fleet data, and (b) considering whether an empty lookup for a hash the node does store should be distinguishable from a genuine miss. This is operational, not a code bug, but it's the highest-impact real-world behavior change.

  2. Low — Backfill runs on every loop iteration
    ensureLookupPartitions is called each 10-min factory pass. In steady state it's two getPartitionsList queries and a no-op, which is fine. Confirm the two list queries are cheap (they are pg_inherits joins, small) — no concern, just noting it's now permanent per-iteration work. The one-time large backfill only happens post-migration.

@darshankabariya

darshankabariya commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Hi @stubbsta,

Medium — Capacity retention still row-deletes from messages_lookup
postgres_driver.nim:1047-1056— deleteOldestMessagesNotWithinLimit (the Capacity retention policy) still runs:

true, it still row-deletes. Here's why we're leaving it for now:

Retention policies stack (our fleets run size:50GB;time:32d together), and no deployment we know adds capacity: to the stack, it's a legacy mode from before time/size existed. It also can't be converted the way the others were: time and size are promises whole hourly partitions can keep, but "exactly the newest N rows" needs row-level deletes by definition — converting it would change what the setting means.

So for this PR: the gap is documented directly on the function, need to create Issue first before deprecate it.

Medium — Silent degraded window after migration / heal

Yes, accepted trade-off, and in practice the window is small: the rebuild reads only the live rows from messages (~137k / ~38 MB after the fleet reindex), so it completes in seconds.

Low — Backfill runs on every loop iteration

Yes, it's very cheap.

Thanks for review.

@darshankabariya darshankabariya added the release-notes Issue/PR needs to be evaluated for inclusion in release notes highlights or upgrade instructions label Aug 14, 2026

@stubbsta stubbsta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fcecin fcecin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@darshankabariya
darshankabariya merged commit bc4a64f into master Aug 19, 2026
17 checks passed
@darshankabariya
darshankabariya deleted the fix_slowquery branch August 19, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes Issue/PR needs to be evaluated for inclusion in release notes highlights or upgrade instructions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: slowQuery in waku.sandbox fleet

4 participants