Files
c-relay-pg/plans/remaining_implementation_plan.md
T

205 lines
8.9 KiB
Markdown

# Remaining Implementation Plan — Zero SQLite on Main Thread + PG Fork Readiness
## Overview
This plan covers all remaining work to achieve two goals:
1. **Zero direct SQLite calls on lws-main** during the event loop
2. **Clean `db_ops` abstraction boundary** so the PG fork has no raw `sqlite3_*` calls outside `db_ops.c`
## Current State
### Completed (Fixes 1-5, 7 + Phases 1-3, 5)
- Global config cache with 5s TTL
- Dedup check moved to event worker
- Async COUNT queries via thread pool
- WoT auth rules cached per-session
- Special-kind EVENT store through async worker
- Subscription logging via fire-and-forget write queue
- IP ban periodic save via write queue
- g_db nulled before event loop
### Remaining
- Fix 6: Reduce reader threads (trivial)
- Fix 8: Move periodic tasks off main thread (partial — IP ban save done, monitoring/status not)
- Fix 9: Cache NIP-11 response
- Sync `store_event()` calls from admin/DM/NIP-09 paths
- Raw `sqlite3_*` calls in `thread_pool.c` and `websockets.c`
- COUNT query path in `websockets.c` still uses `json_each()` instead of `event_tags`
---
## Phase A: Move Remaining Periodic Tasks Off Main Thread
**Goal:** Eliminate the last regular SQLite calls from the lws-main event loop timer.
### A1: Move `generate_and_post_status_event()` to write worker
Currently called synchronously from the main event loop at `websockets.c:3455`.
**Changes:**
- Add `THREAD_POOL_JOB_STATUS_POST` to `thread_pool.h` job type enum
- Add executor in `thread_pool.c` that calls `generate_and_post_status_event()`
- In `websockets.c` main loop, replace direct call with `thread_pool_submit_write()` job
- The write worker has a DB connection, so `generate_stats_json()``store_event()``broadcast_event_to_subscriptions()` all work naturally
- Note: `broadcast_event_to_subscriptions()` is thread-safe (uses its own mutex)
**Files:** `thread_pool.h`, `thread_pool.c`, `websockets.c`
### A2: Move monitoring queries to read/write worker
`monitoring_on_event_stored()` and `monitoring_on_subscription_change()` in `api.c` call `generate_event_driven_monitoring()` / `generate_subscription_driven_monitoring()` which do DB queries and then broadcast ephemeral events.
**Changes:**
- Add `THREAD_POOL_JOB_MONITORING` job type
- Submit monitoring generation as a read job (it mostly does SELECTs)
- The completion broadcasts the ephemeral event via `lws_cancel_service()` + completion queue
- Alternative simpler approach: since these are throttled (default 5s) and only fire when kind 24567 subscribers exist, they could run on the write worker as fire-and-forget
**Files:** `thread_pool.h`, `thread_pool.c`, `api.c`, `websockets.c`
### A3: Move `ip_ban_cleanup()` and `ip_ban_log_stats()` to write worker
Currently called from the 60-second periodic timer at `websockets.c:3479-3480`.
**Changes:**
- Add `THREAD_POOL_JOB_IP_BAN_CLEANUP` job type (or reuse a generic callback job)
- Submit as fire-and-forget write job from the periodic timer
- `ip_ban_cleanup()` does in-memory cleanup + optional DB writes
- `ip_ban_log_stats()` is purely logging, no DB
**Files:** `thread_pool.h`, `thread_pool.c`, `ip_ban.c`, `websockets.c`
---
## Phase B: Route Remaining Sync `store_event()` Calls Through Async Worker
**Goal:** Eliminate all synchronous `store_event()` calls from lws-main context.
### Current sync `store_event()` call sites on main thread
| Call site | File | Context | Frequency |
|-----------|------|---------|-----------|
| Kind 1059 gift wrap store | `websockets.c:1795,1803,1819` | NIP-17 DM processing | Low |
| Kind 14 DM store | `websockets.c:1843` | NIP-17 DM processing | Low |
| Regular event fallback | `websockets.c:1862,1876` | Non-async-eligible events | Low |
| Kind 1059 (auth path) | `websockets.c:2594,2602,2618` | NIP-42 auth + DM | Low |
| Kind 14 (auth path) | `websockets.c:2642` | NIP-42 auth + DM | Low |
| Regular (auth path) | `websockets.c:2661,2675` | NIP-42 auth fallback | Low |
| Admin response store | `config.c:2585` | Admin API response | Very low |
| DM admin gift wrap | `dm_admin.c:389` | Admin DM response | Very low |
| NIP-09 deletion store | `nip009.c:135` | Deletion events | Low |
| Status event store | `api.c:595` | Periodic status post | Very low (handled by Phase A1) |
| Admin response event | `api.c:908` | Admin API | Very low |
| DM chat response | `api.c:1377` | Admin DM chat | Very low |
| Relay-generated event | `api.c:2350` | Relay metadata | Very low |
| DM stats response | `websockets.c:3648` | DM stats command | Very low |
### Strategy
Rather than converting each call site individually, create a **synchronous-to-async bridge**:
**Changes:**
- Create `store_event_async(cJSON* event, store_event_callback_t cb, void* ctx)` that submits the event to the write worker
- For call sites that need the result (e.g., to broadcast after store), use a completion callback
- For fire-and-forget sites (admin responses, DM responses), use NULL callback
- The existing `store_event()` wrapper becomes: submit to write worker + block on completion (for backward compat during transition)
- Eventually, all callers migrate to the async version
**Alternative simpler approach:** Since these are all low-frequency paths, and the write worker already has a DB connection, we can submit them as generic write jobs with a callback that does the store + broadcast. The main thread just needs to hand off the event JSON.
**Files:** `main.c`, `websockets.c`, `api.c`, `config.c`, `dm_admin.c`, `nip009.c`, `thread_pool.h`, `thread_pool.c`
---
## Phase C: `db_ops` Abstraction Cleanup for PG Fork
**Goal:** Ensure all raw `sqlite3_*` calls are inside `db_ops.c` only. External code uses only `db_ops.h` functions.
### C1: Add `db_open_worker_connection()` / `db_close_worker_connection()` to `db_ops.h`
Currently, `thread_pool.c:208` and `websockets.c:537` call `sqlite3_open_v2()` directly.
**Changes:**
- Add to `db_ops.h`:
```c
void* db_open_worker_connection(void); // Returns opaque connection handle
void db_close_worker_connection(void* conn);
```
- Implementation in `db_ops.c`: opens a new SQLite connection with WAL + busy timeout
- `thread_pool.c` worker init calls `db_open_worker_connection()` instead of raw `sqlite3_open_v2()`
- `websockets.c` event-worker calls `db_open_worker_connection()` instead of raw `sqlite3_open_v2()`
- Remove `#include <sqlite3.h>` from `thread_pool.c` and `websockets.c`
**Files:** `db_ops.h`, `db_ops.c`, `thread_pool.c`, `websockets.c`
### C2: Audit and remove remaining `sqlite3` includes outside `db_ops.c`
After C1, grep for any remaining `sqlite3` references outside `db_ops.c` and eliminate them.
**Files:** All `.c` files
### C3: Unify COUNT query tag path
The COUNT query builder in `websockets.c:3849` still uses `json_each(json(tags))` while the REQ query builder in `main.c:1604` uses the indexed `event_tags` table. Unify to use `event_tags` for consistency.
**Files:** `websockets.c`
---
## Phase D: Quick Wins
### D1: Fix 6 — Reduce reader threads from 4 to 1
Change default in `thread_pool_init()` or make configurable. With all reads going through the pool, a single reader is sufficient unless profiling shows otherwise.
**Files:** `thread_pool.c` or config default
### D2: Fix 9 — Cache NIP-11 response
Cache the serialized NIP-11 JSON string with a 60-second TTL. Invalidate on admin config change.
**Files:** `nip011.c`
---
## Implementation Order
| # | Task | Risk | Dependencies |
|---|------|------|-------------|
| 1 | A3: ip_ban_cleanup to write worker | Very Low | None |
| 2 | A1: status post to write worker | Low | None |
| 3 | A2: monitoring to worker | Low | None |
| 4 | D1: reduce reader threads to 1 | Very Low | None |
| 5 | D2: cache NIP-11 response | Very Low | None |
| 6 | C1: db_open/close_worker_connection | Low | None |
| 7 | C3: unify COUNT tag query path | Low | None |
| 8 | C2: audit/remove sqlite3 includes | Very Low | C1 |
| 9 | B: route sync store_event through async | Medium | A1 (status post already handled) |
## Expected Outcome
After all phases:
- **lws-main has zero SQLite calls** during the event loop
- **All `sqlite3_*` calls are inside `db_ops.c`** — clean abstraction boundary
- **PG fork can replace `db_ops.c`** without touching any other file
- Thread model is fully realized:
- `lws-main` = WebSocket I/O + JSON parse + subscription match + completion drain
- `event-worker` = signature verify + dedup + store submission
- `db-read` = REQ queries + COUNT queries + monitoring reads
- `db-write` = event INSERTs + subscription logging + periodic tasks + admin writes
```mermaid
flowchart LR
CLIENT[Nostr Clients] --> LWS[lws-main<br/>Zero SQLite<br/>WS I/O + JSON + Sub Match]
LWS -->|EVENT| EW[event-worker<br/>Verify + Dedup]
EW -->|store job| DW[db-write<br/>INSERT events<br/>Sub logging<br/>Periodic tasks<br/>Admin writes]
EW -->|completion| LWS
DW -->|completion| LWS
LWS -->|REQ/COUNT| DR[db-read<br/>SELECT queries<br/>Monitoring reads]
DR -->|completion| LWS
LWS -->|OK/EVENT/EOSE/COUNT| CLIENT
```