Skip to content

db: add (pubkey, updated_at) index#975

Merged
altafan merged 1 commit intoarkade-os:masterfrom
louisinger:db-index
Mar 18, 2026
Merged

db: add (pubkey, updated_at) index#975
altafan merged 1 commit intoarkade-os:masterfrom
louisinger:db-index

Conversation

@louisinger
Copy link
Collaborator

@louisinger louisinger commented Mar 18, 2026

@sekulicd @altafan please review

Summary by CodeRabbit

  • Chores
    • Added database migrations for performance optimization across supported database systems.

@louisinger louisinger requested a review from sekulicd March 18, 2026 07:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 18, 2026

Walkthrough

This PR adds database migrations to create an index on the vtxo table across PostgreSQL and SQLite databases. The index is created on the (pubkey, updated_at) column combination, with corresponding down-migrations to drop the index if needed.

Changes

Cohort / File(s) Summary
PostgreSQL Migrations
internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.up.sql, internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.down.sql
Adds/drops index idx_vtxo_pubkey_updated_at on vtxo table for (pubkey, updated_at) columns.
SQLite Migrations
internal/infrastructure/db/sqlite/migration/20260318074028_vtxo_indexes.up.sql, internal/infrastructure/db/sqlite/migration/20260318074028_vtxo_indexes.down.sql
Adds/drops index idx_vtxo_pubkey_updated_at on vtxo table for (pubkey, updated_at) columns.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the primary change: adding a database index on (pubkey, updated_at) columns, which matches all four migration files added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arkanaai
Copy link
Contributor

arkanaai bot commented Mar 18, 2026

🔍 Review — db: add (pubkey, updated_at) index

+6 lines, 4 files — Adds a composite index (pubkey, updated_at) on the vtxo table for both Postgres and SQLite.

Assessment: ✅ LGTM

Clean, minimal migration. A few notes:

  • Column order is correct for queries filtering by pubkey then sorting/filtering by updated_at — this matches the common access pattern for fetching a user's VTXOs ordered by recency.
  • IF NOT EXISTS / IF EXISTS guards on both up and down — safe for idempotent re-runs.
  • Postgres & SQLite parity — identical index definition, consistent naming.
  • Related to optimize GetVtxoChain with marker-based bulk preloading #973 — the GetVtxoChain optimization PR benefits from faster pubkey+time lookups, reducing query latency alongside the marker-based bulk preloading.

One consideration: On large deployments with millions of VTXOs, this index will add write overhead on every VTXO insert/update. Given that reads on (pubkey, updated_at) are hot-path (wallet sync, VTXO listing), the tradeoff is clearly worth it — just worth noting for operators running high-throughput servers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.up.sql (1)

1-2: Consider CONCURRENTLY for production deployments with large tables.

The index design is correct for the query patterns in SelectVtxosWithPubkeys and SelectPendingSpentVtxosWithPubkeys. However, standard CREATE INDEX acquires a lock that blocks writes on the vtxo table until completion. For large tables in production, consider using:

CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_vtxo_pubkey_updated_at
    ON vtxo (pubkey, updated_at);

Note: CONCURRENTLY cannot run inside a transaction block, so this may require adjusting how migrations are applied if your migration tool wraps statements in transactions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.up.sql`
around lines 1 - 2, The CREATE INDEX currently defined for
idx_vtxo_pubkey_updated_at on table vtxo will take a write lock on vtxo for the
duration of the operation; update the migration to create the index CONCURRENTLY
(i.e., use CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_vtxo_pubkey_updated_at ON
vtxo (pubkey, updated_at)) so writes are not blocked in production, and ensure
your migration runner does not wrap this statement in a transaction (or split it
out) since CONCURRENTLY cannot run inside a transaction block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.up.sql`:
- Around line 1-2: The CREATE INDEX currently defined for
idx_vtxo_pubkey_updated_at on table vtxo will take a write lock on vtxo for the
duration of the operation; update the migration to create the index CONCURRENTLY
(i.e., use CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_vtxo_pubkey_updated_at ON
vtxo (pubkey, updated_at)) so writes are not blocked in production, and ensure
your migration runner does not wrap this statement in a transaction (or split it
out) since CONCURRENTLY cannot run inside a transaction block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8b64db2-83a1-4c9b-8962-bf11e0b35724

📥 Commits

Reviewing files that changed from the base of the PR and between bb633d8 and 832a065.

📒 Files selected for processing (4)
  • internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.down.sql
  • internal/infrastructure/db/postgres/migration/20260318074150_vtxo_indexes.up.sql
  • internal/infrastructure/db/sqlite/migration/20260318074028_vtxo_indexes.down.sql
  • internal/infrastructure/db/sqlite/migration/20260318074028_vtxo_indexes.up.sql

@altafan altafan merged commit 88d48e0 into arkade-os:master Mar 18, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants