Skip to content

fix(cold-sql): pool starvation and query inefficiencies#45

Merged
prestwich merged 7 commits intomainfrom
prestwich/fix-cold-sql-pool-and-queries
Apr 6, 2026
Merged

fix(cold-sql): pool starvation and query inefficiencies#45
prestwich merged 7 commits intomainfrom
prestwich/fix-cold-sql-pool-and-queries

Conversation

@prestwich
Copy link
Copy Markdown
Member

@prestwich prestwich commented Apr 3, 2026

Summary

Fixes ENG-2124

  • Pool configuration: Replace custom PoolOverrides with sqlx::pool::PoolOptions<Any>, giving callers the full sqlx pool configuration surface. Only force max_connections = 1 for in-memory SQLite URLs (where per-connection state isolation requires it); file-backed SQLite and PostgreSQL honor caller-provided options as-is. Adds SqlConnector::with_pool_options builder method for production tunability.
  • get_logs optimization: Replace COUNT(*) + SELECT with single LIMIT query — faster in both under-limit and over-limit cases.
  • get_receipt consolidation: Merge 5 sequential queries (header, receipt+tx, logs, prior gas) into 2-3 via combined JOIN with COALESCE subquery.
  • drain_above batch override: Replace default N+1 implementation (3N+7 queries) with batched single-transaction approach (9 queries regardless of block count).
  • Integer safety: Debug assertions on to_i64/from_i64, checked tx_type conversion throughout.

Test plan

  • cargo t -p signet-cold-sql --all-features — SQLite + PG conformance passing
  • cargo clippy -p signet-cold-sql --all-features --all-targets — clean
  • cargo clippy -p signet-cold-sql --no-default-features --all-targets — clean
  • cargo clippy --workspace --all-features --all-targets — clean
  • cargo +nightly fmt — clean
  • ./scripts/test-postgres.sh — PostgreSQL conformance against real database

Review Fixes

  • Use from_i64 for prior_cumulative_gas instead of raw as u64 cast (bypassed debug assertion)
  • Use actual tx_index from DB row instead of enumerate index in drain_above
  • Read first_log_index from DB instead of recomputing from log counts in drain_above
  • Add debug_assert on gas_used subtraction to detect cumulative gas corruption
  • Import BTreeMap at file top instead of inline fully-qualified paths
  • Remove dead r_tx_index column from combined get_receipt query
  • Extract shared delete_above_in_tx helper from truncate_above/drain_above
  • Add comprehensive field assertions to drain_above conformance test (transaction_index, first_log_index, gas_used, tx_hash, from, block metadata)
  • Enhance get_receipts_in_block and receipt lookup conformance assertions

Reviewed and updated by Claude Code.

🤖 Generated with Claude Code

prestwich added a commit to init4tech/node-components that referenced this pull request Apr 3, 2026
Update all signet-storage family crates (signet-storage, signet-cold,
signet-hot, signet-hot-mdbx, signet-cold-mdbx, signet-storage-types)
to 0.7 from init4tech/storage#45. Bump workspace version to 0.17.1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@prestwich prestwich requested review from Evalir and Fraser999 April 6, 2026 16:13
- Add per-backend pool defaults and SqlConnector builder
- Add safety checks to integer conversions
- Replace get_logs COUNT+SELECT with single LIMIT query
- Consolidate get_receipt and batch drain_above
- Bump workspace version to 0.7.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@prestwich prestwich force-pushed the prestwich/fix-cold-sql-pool-and-queries branch from c4a8b9e to b513020 Compare April 6, 2026 16:15
@prestwich prestwich enabled auto-merge (squash) April 6, 2026 16:47
prestwich and others added 4 commits April 6, 2026 12:53
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the custom `PoolOverrides` struct and accept
`sqlx::pool::PoolOptions<Any>` directly, giving callers the full sqlx
pool configuration surface. Only force `max_connections = 1` for
in-memory SQLite URLs where per-connection state isolation requires it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add convenience builder methods on SqlConnector for the most common
pool settings: max_connections, min_connections, acquire_timeout,
max_lifetime, and idle_timeout. The full PoolOptions can still be
set via with_pool_options.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use from_i64 for prior_cumulative_gas (was bypassing debug assertion)
- Use actual tx_index from DB row instead of enumerate index in drain_above
- Read first_log_index from DB instead of recomputing in drain_above
- Add debug_assert on gas_used subtraction for corruption detection
- Import BTreeMap at file top instead of inline fully-qualified paths
- Remove dead r_tx_index column from combined get_receipt query
- Extract shared delete_above_in_tx helper from truncate/drain_above

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
prestwich and others added 2 commits April 6, 2026 13:44
Enhance conformance tests to verify receipt properties that were
previously only checked as counts:

- test_drain_above: assert transaction_index, first_log_index (via
  log_index), gas_used, tx_hash, from, block_hash, block_timestamp
  on all drained receipts across multiple blocks
- test_cold_receipt_metadata: verify all fields on get_receipts_in_block
  results (was only checking gas_used)
- test_confirmation_metadata: add tx_hash and from assertions on
  receipt lookups

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace `r.*` with explicit column lists in `drain_above` and
`get_receipts_in_block` to avoid potential column name collisions
from the JOIN with the transactions table.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@prestwich prestwich merged commit ecdeb02 into main Apr 6, 2026
6 checks passed
prestwich added a commit to init4tech/node-components that referenced this pull request Apr 8, 2026
* chore: bump signet-storage to 0.7 and workspace to 0.17.1

Update all signet-storage family crates (signet-storage, signet-cold,
signet-hot, signet-hot-mdbx, signet-cold-mdbx, signet-storage-types)
to 0.7 from init4tech/storage#45. Bump workspace version to 0.17.1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: switch signet-storage deps from git to crates.io 0.7

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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