Skip to content

Commit 9ca3367

Browse files
committed
chore: addressed review comments (#68)
1 parent 11f2180 commit 9ca3367

8 files changed

Lines changed: 441 additions & 37 deletions

File tree

CONFIGURATION.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,27 @@ The schema ships with a small, query-driven set of indexes. The most important o
8484

8585
| Index | Covers |
8686
|----------------------------------------------|----------------------------------------------------------------------------------------------------------|
87-
| `events_active_pubkey_kind_created_at_idx` | `REQ` with `authors`+`kinds` ordered by `created_at DESC`; `hasActiveRequestToVanish`; by-pubkey deletes. Partial on `deleted_at IS NULL`. |
87+
| `events_active_pubkey_kind_created_at_idx` | `REQ` with `authors`+`kinds` ordered by `created_at DESC, event_id ASC`; `hasActiveRequestToVanish`; by-pubkey deletes. Composite key `(event_pubkey, event_kind, event_created_at DESC, event_id)` so the ORDER BY tie-breaker is satisfied from the index without a sort step. |
8888
| `events_deleted_at_partial_idx` | Retention purge over soft-deleted rows. Partial on `deleted_at IS NOT NULL`. |
89-
| `invoices_pending_created_at_idx` | `findPendingInvoices` poll. Partial on `status = 'pending'`. |
89+
| `invoices_pending_created_at_idx` | `findPendingInvoices` poll (`ORDER BY created_at ASC`). Partial on `status = 'pending'`. |
9090
| `event_tags (tag_name, tag_value)` | NIP-01 generic tag filters (`#e`, `#p`, …) via the normalized `event_tags` table. |
9191
| `events_event_created_at_index` | Time-range scans (`since` / `until`). |
9292
| `events_event_kind_index` | Kind-only filters and purge kind-whitelist logic. |
9393

9494
Run the read-only benchmark against your own database to confirm the planner is using the expected indexes and to record baseline latencies:
9595

9696
```sh
97-
NODE_OPTIONS="-r dotenv/config" npm run db:benchmark
98-
NODE_OPTIONS="-r dotenv/config" npm run db:benchmark -- --runs 5 --kind 1 --limit 500
97+
npm run db:benchmark
98+
npm run db:benchmark -- --runs 5 --kind 1 --limit 500
9999
```
100100

101-
The benchmark issues only `EXPLAIN (ANALYZE, BUFFERS)` and `SELECT` statements — it never writes. Flags: `--runs <n>` (default 3), `--kind <n>` (default 1 / `TEXT_NOTE`), `--limit <n>` (default 500), `--horizon-days <n>` (default 7), `--help`.
101+
The `db:benchmark` script loads the local `.env` file automatically (via `node --env-file-if-exists=.env`), using the same `DB_HOST`/`DB_PORT`/`DB_USER`/`DB_PASSWORD`/`DB_NAME` variables as the relay. The benchmark issues only `EXPLAIN (ANALYZE, BUFFERS)` and `SELECT` statements — it never writes. Flags: `--runs <n>` (default 3), `--kind <n>` (default 1 / `TEXT_NOTE`; pass `0` for SET_METADATA), `--limit <n>` (default 500), `--horizon-days <n>` (default 7), `--help`.
102+
103+
For a full before/after proof of the index impact (seeds a throwaway dataset, drops and recreates the indexes, and prints a BEFORE/AFTER table), use:
104+
105+
```sh
106+
npm run db:verify-index-impact
107+
```
102108

103109
The hot-path index migration (`20260420_120000_add_hot_path_indexes.js`) uses `CREATE INDEX CONCURRENTLY`, so it can be applied to a running relay without taking `ACCESS EXCLUSIVE` locks on the `events` or `invoices` tables.
104110

README.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,15 @@ npm run db:benchmark
660660
npm run db:benchmark -- --runs 5 --kind 1 --limit 500
661661
```
662662

663-
The benchmark only issues `EXPLAIN (ANALYZE, BUFFERS)` and `SELECT` statements against your configured database — it never writes. Use it to confirm the `events_active_pubkey_kind_created_at_idx`, `events_deleted_at_partial_idx`, and `invoices_pending_created_at_idx` indexes are being picked up. See the *Database indexes and benchmarking* section of [CONFIGURATION.md](CONFIGURATION.md).
663+
The benchmark only issues `EXPLAIN (ANALYZE, BUFFERS)` and `SELECT` statements against your configured database — it never writes. It loads `DB_*` variables from `.env` automatically (via `node --env-file-if-exists=.env`), so no extra setup is required beyond the one you already need to run the relay. Use it to confirm the `events_active_pubkey_kind_created_at_idx`, `events_deleted_at_partial_idx`, and `invoices_pending_created_at_idx` indexes are being picked up.
664+
665+
For a reproducible before/after proof on a throwaway dataset, run:
666+
667+
```
668+
npm run db:verify-index-impact
669+
```
670+
671+
It seeds ~200k synthetic events, drops the hot-path indexes, runs EXPLAIN (ANALYZE, BUFFERS) for each hot query, recreates the indexes, and prints a BEFORE/AFTER table. See the *Database indexes and benchmarking* section of [CONFIGURATION.md](CONFIGURATION.md).
664672

665673
## Relay Maintenance
666674

migrations/20260420_120000_add_hot_path_indexes.js

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,41 +14,54 @@
1414
exports.config = { transaction: false }
1515

1616
exports.up = async function (knex) {
17-
// Covers the hottest write-adjacent reads:
17+
// Covers the hottest subscription / per-message reads:
1818
//
19-
// 1. `EventRepository.hasActiveRequestToVanish(pubkey)`
19+
// 1. NIP-01 REQ with `authors` + `kinds` ordered by created_at DESC
20+
// (see EventRepository.findByFilters):
21+
// WHERE event_pubkey = ? AND event_kind IN (...)
22+
// ORDER BY event_created_at DESC, event_id ASC LIMIT N
23+
//
24+
// 2. `EventRepository.hasActiveRequestToVanish(pubkey)` — invoked on every
25+
// inbound event via UserRepository.isVanished:
2026
// WHERE event_pubkey = ? AND event_kind = 62 AND deleted_at IS NULL
21-
// -- invoked on every inbound event via UserRepository.isVanished
2227
//
23-
// 2. `EventRepository.deleteByPubkeyExceptKinds(pubkey, kinds)`
28+
// 3. `EventRepository.deleteByPubkeyExceptKinds(pubkey, kinds)`:
2429
// WHERE event_pubkey = ? AND event_kind NOT IN (...) AND deleted_at IS NULL
2530
//
26-
// 3. NIP-01 REQ with `authors` + `kinds` filters ordered by created_at:
27-
// WHERE event_pubkey IN (...) AND event_kind IN (...)
28-
// ORDER BY event_created_at DESC LIMIT N
31+
// The index is intentionally NOT partial on `deleted_at IS NULL`: the REQ
32+
// subscription path in findByFilters does not currently add that predicate,
33+
// so a partial index would be ineligible for the most important query shape.
34+
// Soft-deleted rows are a small fraction of total rows in practice (they get
35+
// hard-deleted by the retention sweep), so the bloat is negligible compared
36+
// to the benefit of the index being usable by the hot path.
2937
//
30-
// Partial on `deleted_at IS NULL` so soft-deleted rows never bloat the index.
31-
// DESC on event_created_at lets the planner satisfy LIMIT N without a sort.
38+
// Including `event_id` as the final column makes the composite key match the
39+
// full ORDER BY (created_at DESC, event_id ASC) used by findByFilters, so the
40+
// planner can satisfy LIMIT N directly from the index without an extra sort
41+
// step for the tie-breaker.
3242
await knex.raw(`
3343
CREATE INDEX CONCURRENTLY IF NOT EXISTS events_active_pubkey_kind_created_at_idx
34-
ON events (event_pubkey, event_kind, event_created_at DESC)
35-
WHERE deleted_at IS NULL
44+
ON events (event_pubkey, event_kind, event_created_at DESC, event_id)
3645
`)
3746

38-
// Supports the retention/purge scan in `deleteExpiredAndRetained`:
47+
// Supports the retention / purge scan in `deleteExpiredAndRetained` and the
48+
// vanish hard-delete follow-up:
3949
// WHERE deleted_at IS NOT NULL
40-
// Partial index is tiny because well-maintained relays hard-delete these rows
41-
// periodically and most events have deleted_at IS NULL.
50+
// Partial index is tiny because well-maintained relays hard-delete these
51+
// rows periodically and the vast majority of events have deleted_at IS NULL.
4252
await knex.raw(`
4353
CREATE INDEX CONCURRENTLY IF NOT EXISTS events_deleted_at_partial_idx
4454
ON events (deleted_at)
4555
WHERE deleted_at IS NOT NULL
4656
`)
4757

48-
// Supports `InvoiceRepository.findPendingInvoices` which is polled by the
49-
// maintenance worker:
50-
// WHERE status = 'pending' ORDER BY created_at
51-
// Partial on status='pending' so the index only contains the rows we scan.
58+
// Supports `InvoiceRepository.findPendingInvoices`, which is polled by the
59+
// maintenance worker to detect settled invoices:
60+
// WHERE status = 'pending' ORDER BY created_at ASC OFFSET ? LIMIT ?
61+
// Partial on status = 'pending' so the index only contains the rows the
62+
// poller actually scans. Keyed on `created_at` so the planner can satisfy
63+
// the ORDER BY straight from the index (FIFO polling, bounded tail latency
64+
// even with large pending backlogs).
5265
await knex.raw(`
5366
CREATE INDEX CONCURRENTLY IF NOT EXISTS invoices_pending_created_at_idx
5467
ON invoices (created_at)

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
"db:migrate": "knex migrate:latest",
4444
"db:migrate:rollback": "knex migrate:rollback",
4545
"db:seed": "knex seed:run",
46-
"db:benchmark": "node -r ts-node/register src/scripts/benchmark-queries.ts",
46+
"db:benchmark": "node --env-file-if-exists=.env -r ts-node/register src/scripts/benchmark-queries.ts",
47+
"db:verify-index-impact": "node --env-file-if-exists=.env -r ts-node/register scripts/verify-index-impact.ts",
4748
"pretest:unit": "node -e \"require('fs').mkdirSync('.test-reports/unit', {recursive: true})\"",
4849
"test:unit": "mocha 'test/**/*.spec.ts'",
4950
"test:unit:watch": "npm run test:unit -- --min --watch --watch-files src/**/*,test/**/*",

0 commit comments

Comments
 (0)