diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index b8accd33..3146007d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -191,6 +191,12 @@ jobs: go build -o ../../corescope-server . echo "Go server built successfully" + - name: Build Go migrate tool + run: | + cd cmd/migrate + go build -o ../../corescope-migrate . + echo "Go migrate tool built successfully" + - name: Install npm dependencies run: npm ci --production=false @@ -205,6 +211,15 @@ jobs: - name: Freshen fixture timestamps run: bash tools/freshen-fixture.sh test-fixtures/e2e-fixture.db + - name: Migrate fixture DB to current schema (#1287) + # Server now ASSERTs schema is migrated and refuses to start + # otherwise (cmd/server/main.go: dbschema.AssertReady). In prod + # the ingestor owns dbschema.Apply, but CI starts only the + # server against the committed e2e fixture — so we run the + # standalone migrate tool here to bring the fixture up to the + # required shape before the server boots. + run: ./corescope-migrate -db test-fixtures/e2e-fixture.db + - name: Start Go server with fixture DB run: | fuser -k 13581/tcp 2>/dev/null || true diff --git a/cmd/ingestor/config.go b/cmd/ingestor/config.go index 8f6b8431..ba4921fd 100644 --- a/cmd/ingestor/config.go +++ b/cmd/ingestor/config.go @@ -75,6 +75,18 @@ type Config struct { // obsBlacklistSetCached is the lazily-built lowercase set for O(1) lookups. obsBlacklistSetCached map[string]bool obsBlacklistOnce sync.Once + + // NeighborEdgesMaxAgeDays controls neighbor_edges row retention + // (#1287 — moved from cmd/server). 0 = default 5. + NeighborEdgesMaxAgeDays int `json:"neighborEdgesMaxAgeDays,omitempty"` +} + +// NeighborEdgesDaysOrDefault returns the configured pruning window or 5. +func (c *Config) NeighborEdgesDaysOrDefault() int { + if c == nil || c.NeighborEdgesMaxAgeDays <= 0 { + return 5 + } + return c.NeighborEdgesMaxAgeDays } // GeoFilterConfig is an alias for the shared geofilter.Config type. diff --git a/cmd/ingestor/db.go b/cmd/ingestor/db.go index 5304e33c..a5b38bc8 100644 --- a/cmd/ingestor/db.go +++ b/cmd/ingestor/db.go @@ -12,6 +12,7 @@ import ( "sync/atomic" "time" + "github.com/meshcore-analyzer/dbschema" "github.com/meshcore-analyzer/packetpath" _ "modernc.org/sqlite" ) @@ -110,6 +111,13 @@ func OpenStoreWithInterval(dbPath string, sampleIntervalSec int) (*Store, error) return nil, fmt.Errorf("applying schema: %w", err) } + // Apply the additional server-originated migrations (now owned by + // the ingestor per #1287). Adds the indexes/columns that used to live + // in cmd/server/ensure_*.go: server now ASSERTS these exist. + if err := dbschema.Apply(db, log.Printf); err != nil { + return nil, fmt.Errorf("dbschema.Apply: %w", err) + } + s := &Store{db: db, sampleIntervalSec: sampleIntervalSec} if err := s.prepareStatements(); err != nil { return nil, fmt.Errorf("preparing statements: %w", err) diff --git a/cmd/ingestor/go.mod b/cmd/ingestor/go.mod index f5bdf468..24987cbb 100644 --- a/cmd/ingestor/go.mod +++ b/cmd/ingestor/go.mod @@ -25,6 +25,10 @@ require github.com/meshcore-analyzer/perfio v0.0.0 replace github.com/meshcore-analyzer/perfio => ../../internal/perfio +require github.com/meshcore-analyzer/dbschema v0.0.0 + +replace github.com/meshcore-analyzer/dbschema => ../../internal/dbschema + require ( github.com/dustin/go-humanize v1.0.1 // indirect github.com/google/uuid v1.6.0 // indirect diff --git a/cmd/ingestor/main.go b/cmd/ingestor/main.go index 41c000e6..1f62a01c 100644 --- a/cmd/ingestor/main.go +++ b/cmd/ingestor/main.go @@ -62,6 +62,15 @@ func main() { // Async backfill: path_json from raw_hex (#888) — must not block MQTT startup store.BackfillPathJSONAsync() + // Soft-delete blacklisted observers (#1287 — moved from cmd/server). + if len(cfg.ObserverBlacklist) > 0 { + store.SoftDeleteBlacklistedObservers(cfg.ObserverBlacklist) + } + + // Async backfill: from_pubkey for legacy ADVERT rows (#1143). + // Moved from cmd/server in #1287. Best-effort; must not block MQTT. + go store.BackfillFromPubkey(5000, 100*time.Millisecond, nil) + // Check auto_vacuum mode and optionally migrate (#919) store.CheckAutoVacuum(cfg) @@ -140,6 +149,28 @@ func main() { log.Printf("[prune] auto-prune enabled: packets older than %d days will be removed daily", packetDays) } + // Daily neighbor_edges retention (#1287 — moved from cmd/server). + { + nDays := cfg.NeighborEdgesDaysOrDefault() + neighborPruneTicker := time.NewTicker(24 * time.Hour) + go func() { + time.Sleep(4 * time.Minute) // stagger + if n, err := store.PruneNeighborEdges(nDays); err != nil { + log.Printf("[neighbor-prune] error: %v", err) + } else if n > 0 { + log.Printf("[neighbor-prune] startup pruned %d edges older than %d days", n, nDays) + } + for range neighborPruneTicker.C { + if n, err := store.PruneNeighborEdges(nDays); err != nil { + log.Printf("[neighbor-prune] error: %v", err) + } else if n > 0 { + log.Printf("[neighbor-prune] pruned %d edges older than %d days", n, nDays) + } + } + }() + log.Printf("[neighbor-prune] auto-prune enabled: edges older than %d days", nDays) + } + // Periodic stats logging (every 5 minutes) statsTicker := time.NewTicker(5 * time.Minute) go func() { @@ -152,6 +183,13 @@ func main() { // endpoint (#1120). Best-effort; never fatal. StartStatsFileWriter(store, time.Second) + // Neighbor-edges builder (#1287 — Option 4): ingestor owns + // neighbor_edges writes. Runs every 60s. Server reads the snapshot + // via cmd/server/neighbor_recomputer.go on the same cadence. + stopNeighborBuilder := store.StartNeighborEdgesBuilder(NeighborEdgesBuilderInterval) + defer stopNeighborBuilder() + log.Printf("[neighbor-build] enabled (interval=%s)", NeighborEdgesBuilderInterval) + channelKeys := loadChannelKeys(cfg, *configPath) if len(channelKeys) > 0 { log.Printf("Loaded %d channel keys for GRP_TXT decryption", len(channelKeys)) diff --git a/cmd/ingestor/maintenance.go b/cmd/ingestor/maintenance.go index 44c8f7aa..44d52eec 100644 --- a/cmd/ingestor/maintenance.go +++ b/cmd/ingestor/maintenance.go @@ -1,9 +1,13 @@ package main import ( + "database/sql" + "encoding/json" "fmt" "log" "time" + + "github.com/meshcore-analyzer/dbschema" ) // PruneOldPackets deletes transmissions (and their child observations) @@ -44,3 +48,175 @@ func (s *Store) PruneOldPackets(days int) (int64, error) { } return n, nil } + +// SoftDeleteBlacklistedObservers marks observers in the blacklist as +// inactive=1 so they are hidden from API responses. Owned by ingestor +// per #1287. Runs once at startup. +func (s *Store) SoftDeleteBlacklistedObservers(blacklist []string) { + n, err := dbschema.SoftDeleteBlacklistedObservers(s.db, blacklist) + if err != nil { + log.Printf("[observer-blacklist] warning: soft-delete failed: %v", err) + return + } + if n > 0 { + log.Printf("[observer-blacklist] soft-deleted %d blacklisted observer(s)", n) + } +} + +// PruneNeighborEdges deletes rows older than maxAgeDays from +// neighbor_edges. Owned by the ingestor per #1287 (was in cmd/server). +// Returns DB rows deleted. +func (s *Store) PruneNeighborEdges(maxAgeDays int) (int64, error) { + if maxAgeDays <= 0 { + return 0, nil + } + cutoff := time.Now().UTC().Add(-time.Duration(maxAgeDays) * 24 * time.Hour).Format(time.RFC3339) + res, err := s.db.Exec("DELETE FROM neighbor_edges WHERE last_seen < ?", cutoff) + if err != nil { + return 0, fmt.Errorf("prune neighbor_edges: %w", err) + } + n, _ := res.RowsAffected() + if n > 0 { + log.Printf("[neighbor-prune] removed %d DB rows older than %d days", n, maxAgeDays) + } + return n, nil +} + +// ─── from_pubkey backfill (#1143) ────────────────────────────────────────── +// +// Moved from cmd/server/from_pubkey_migration.go in #1287. Runs from the +// ingestor's maintenance loop. Populates transmissions.from_pubkey for +// ADVERT rows whose value is still NULL, by parsing decoded_json.pubKey. + +// FromPubkeyBackfillStats holds progress for /api/healthz exposure. +// The ingestor exposes these via stats_file.go so the server can read +// them without writing. +type FromPubkeyBackfillStats struct { + Total int64 `json:"total"` + Processed int64 `json:"processed"` + Done bool `json:"done"` +} + +// BackfillFromPubkey scans transmissions where from_pubkey IS NULL and +// payload_type = 4 (ADVERT) and populates from_pubkey from decoded_json. +// Chunked + yields between batches. Safe to call repeatedly; once a row +// is set to either "" or hex it never matches the WHERE clause again. +func (s *Store) BackfillFromPubkey(chunkSize int, yieldDuration time.Duration, progress func(total, processed int64, done bool)) { + defer func() { + if r := recover(); r != nil { + log.Printf("[backfill] from_pubkey panic recovered: %v", r) + } + if progress != nil { + progress(0, 0, true) // signal done; values overwritten below if collected + } + }() + if chunkSize <= 0 { + chunkSize = 5000 + } + + var total int64 + if err := s.db.QueryRow( + "SELECT COUNT(*) FROM transmissions WHERE from_pubkey IS NULL AND payload_type = 4", + ).Scan(&total); err != nil { + log.Printf("[backfill] from_pubkey count error: %v", err) + return + } + if total == 0 { + log.Println("[backfill] from_pubkey: nothing to do") + if progress != nil { + progress(0, 0, true) + } + return + } + if progress != nil { + progress(total, 0, false) + } + log.Printf("[backfill] from_pubkey starting: %d ADVERT rows", total) + + stmt, err := s.db.Prepare("UPDATE transmissions SET from_pubkey = ? WHERE id = ?") + if err != nil { + log.Printf("[backfill] from_pubkey prepare: %v", err) + return + } + defer stmt.Close() + + var processed int64 + for { + rows, err := s.db.Query( + "SELECT id, decoded_json FROM transmissions WHERE from_pubkey IS NULL AND payload_type = 4 LIMIT ?", + chunkSize) + if err != nil { + log.Printf("[backfill] from_pubkey select: %v", err) + return + } + type row struct { + id int64 + pk string + } + batch := make([]row, 0, chunkSize) + for rows.Next() { + var id int64 + var dj sql.NullString + if err := rows.Scan(&id, &dj); err != nil { + continue + } + batch = append(batch, row{id: id, pk: extractPubkeyFromAdvertJSON(dj.String)}) + } + rows.Close() + if len(batch) == 0 { + break + } + + tx, err := s.db.Begin() + if err != nil { + log.Printf("[backfill] from_pubkey begin tx: %v", err) + return + } + txStmt := tx.Stmt(stmt) + for _, b := range batch { + // Sentinel: "" = scanned-no-pubkey (so the WHERE clause + // won't keep rescanning this row). hex = real pubkey. + var val interface{} = "" + if b.pk != "" { + val = b.pk + } + if _, err := txStmt.Exec(val, b.id); err != nil { + log.Printf("[backfill] from_pubkey update id=%d: %v", b.id, err) + } + } + if err := tx.Commit(); err != nil { + log.Printf("[backfill] from_pubkey commit: %v", err) + return + } + processed += int64(len(batch)) + if progress != nil { + progress(total, processed, false) + } + if len(batch) < chunkSize { + break + } + if yieldDuration > 0 { + time.Sleep(yieldDuration) + } + } + log.Printf("[backfill] from_pubkey complete: %d rows processed", processed) + if progress != nil { + progress(total, processed, true) + } +} + +// extractPubkeyFromAdvertJSON parses an ADVERT decoded_json blob and +// returns the pubKey field, or "" if absent/invalid. +func extractPubkeyFromAdvertJSON(s string) string { + if s == "" { + return "" + } + var m map[string]interface{} + if err := json.Unmarshal([]byte(s), &m); err != nil { + return "" + } + if v, ok := m["pubKey"].(string); ok { + return v + } + return "" +} diff --git a/cmd/ingestor/neighbor_builder.go b/cmd/ingestor/neighbor_builder.go new file mode 100644 index 00000000..ff93d9df --- /dev/null +++ b/cmd/ingestor/neighbor_builder.go @@ -0,0 +1,246 @@ +package main + +import ( + "database/sql" + "encoding/json" + "fmt" + "log" + "strings" + "sync" + "time" +) + +// NeighborEdgesBuilderInterval is how often the ingestor rescans +// observations and refreshes neighbor_edges. Server reads with the +// same 60s cadence (see cmd/server/neighbor_recomputer.go); a 60s +// pulse here is sufficient to keep the snapshot fresh. +const NeighborEdgesBuilderInterval = 60 * time.Second + +// payloadADVERT mirrors the constant in cmd/server/decoder.go. +// Duplicated rather than imported so the ingestor binary stays +// independent of the server package. +const payloadADVERT = 0x04 + +// edgeRow is one row to upsert into neighbor_edges. (a, b) is already +// canonical-ordered (a <= b). +type edgeRow struct { + a, b, ts string +} + +// StartNeighborEdgesBuilder launches the periodic builder. On each +// tick it rescans recent observations + transmissions and upserts +// derived neighbor_edges rows. Builder is the only writer to +// neighbor_edges (#1287). +// +// The function returns a stop closure. Initial build runs synchronously +// before the ticker starts so the server's first snapshot load picks +// up real data instead of an empty table. +func (s *Store) StartNeighborEdgesBuilder(interval time.Duration) func() { + if interval <= 0 { + interval = NeighborEdgesBuilderInterval + } + stop := make(chan struct{}) + done := make(chan struct{}) + + // Synchronous warm-up: a single pass so the first server load + // after process start sees a populated table. + if n, err := s.buildAndPersistNeighborEdges(); err != nil { + log.Printf("[neighbor-build] initial build error: %v", err) + } else { + log.Printf("[neighbor-build] initial build: %d edges upserted", n) + } + + var stopOnce sync.Once + go func() { + defer close(done) + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-t.C: + if n, err := s.buildAndPersistNeighborEdges(); err != nil { + log.Printf("[neighbor-build] tick error: %v", err) + } else if n > 0 { + log.Printf("[neighbor-build] %d edges upserted", n) + } + case <-stop: + return + } + } + }() + + return func() { + stopOnce.Do(func() { close(stop) }) + select { + case <-done: + case <-time.After(5 * time.Second): + } + } +} + +// buildAndPersistNeighborEdges scans transmissions + observations, +// extracts edge candidates (originator↔first-hop on ADVERTs; +// observer↔last-hop on all packet types) and upserts them into +// neighbor_edges. Returns count of attempted upserts. +// +// Resolution of hop-prefix → full pubkey is done via a one-shot +// SELECT of (lowered) pubkey prefixes from nodes. Prefixes with +// multiple candidates are skipped (matches the conservative +// resolution rule in cmd/server/extractEdgesFromObs). +func (s *Store) buildAndPersistNeighborEdges() (int, error) { + prefixIdx, err := buildPrefixIndex(s.db) + if err != nil { + return 0, fmt.Errorf("build prefix index: %w", err) + } + + rows, err := s.db.Query(`SELECT + t.payload_type, + t.decoded_json, + COALESCE(t.from_pubkey, ''), + COALESCE(o.path_json, ''), + COALESCE(obs.id, '') AS observer_id, + o.timestamp + FROM observations o + JOIN transmissions t ON t.id = o.transmission_id + LEFT JOIN observers obs ON obs.rowid = o.observer_idx`) + if err != nil { + return 0, fmt.Errorf("scan observations: %w", err) + } + defer rows.Close() + + var edges []edgeRow + for rows.Next() { + var payloadType sql.NullInt64 + var decodedJSON, fromPubkey, pathJSON, observerID string + var epochTs int64 + if err := rows.Scan(&payloadType, &decodedJSON, &fromPubkey, &pathJSON, &observerID, &epochTs); err != nil { + continue + } + fromNode := strings.ToLower(fromPubkey) + if fromNode == "" { + fromNode = strings.ToLower(extractPubkeyFromAdvertJSON(decodedJSON)) + } + isAdvert := payloadType.Valid && payloadType.Int64 == int64(payloadADVERT) + ts := time.Unix(epochTs, 0).UTC().Format(time.RFC3339) + observerPK := strings.ToLower(observerID) + path := parsePathArray(pathJSON) + + if len(path) == 0 { + if isAdvert && fromNode != "" && fromNode != observerPK && observerPK != "" { + edges = append(edges, canonEdge(fromNode, observerPK, ts)) + } + continue + } + if isAdvert && fromNode != "" { + if resolved, ok := resolvePrefix(prefixIdx, path[0]); ok && resolved != fromNode { + edges = append(edges, canonEdge(fromNode, resolved, ts)) + } + } + if observerPK != "" { + last := path[len(path)-1] + if resolved, ok := resolvePrefix(prefixIdx, last); ok && resolved != observerPK { + edges = append(edges, canonEdge(observerPK, resolved, ts)) + } + } + } + + if len(edges) == 0 { + return 0, nil + } + + tx, err := s.db.Begin() + if err != nil { + return 0, fmt.Errorf("begin: %w", err) + } + defer tx.Rollback() + stmt, err := tx.Prepare(`INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) + VALUES (?, ?, 1, ?) + ON CONFLICT(node_a, node_b) DO UPDATE SET + count = count + 1, + last_seen = MAX(last_seen, excluded.last_seen)`) + if err != nil { + return 0, fmt.Errorf("prepare: %w", err) + } + defer stmt.Close() + var firstErr error + for _, e := range edges { + if _, err := stmt.Exec(e.a, e.b, e.ts); err != nil && firstErr == nil { + firstErr = err + } + } + if firstErr != nil { + return 0, fmt.Errorf("upsert: %w", firstErr) + } + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("commit: %w", err) + } + return len(edges), nil +} + +// canonEdge orders the pair so node_a <= node_b (matches the existing +// schema convention used by the loader and the bridge recomputer). +func canonEdge(a, b, ts string) edgeRow { + if a > b { + a, b = b, a + } + return edgeRow{a, b, ts} +} + +// parsePathArray returns the hop strings from a path_json blob. +// Defensive against missing/invalid JSON. +func parsePathArray(s string) []string { + if s == "" || s == "[]" { + return nil + } + var arr []string + if json.Unmarshal([]byte(s), &arr) != nil { + return nil + } + return arr +} + +// prefixIndex maps a hop prefix (lowercase) → all full pubkeys whose +// public_key starts with that prefix. Prefixes with > 1 candidate are +// considered ambiguous and skipped during resolution. +type prefixIndex map[string][]string + +// buildPrefixIndex reads nodes.public_key and builds the prefix → pubkey +// map. We index every 1-byte (2 hex char) prefix length the firmware +// uses (1, 2, 3, 4, 6, 8). Memory cost is O(nodes × len(prefixLens)). +func buildPrefixIndex(db *sql.DB) (prefixIndex, error) { + rows, err := db.Query(`SELECT public_key FROM nodes`) + if err != nil { + return nil, err + } + defer rows.Close() + idx := make(prefixIndex, 1024) + var prefixLens = []int{1 * 2, 2 * 2, 3 * 2, 4 * 2, 6 * 2, 8 * 2} + for rows.Next() { + var pk string + if err := rows.Scan(&pk); err != nil { + continue + } + pkLower := strings.ToLower(pk) + for _, n := range prefixLens { + if len(pkLower) < n { + continue + } + prefix := pkLower[:n] + idx[prefix] = append(idx[prefix], pkLower) + } + } + return idx, nil +} + +// resolvePrefix returns the single resolved pubkey if exactly one +// candidate matches, otherwise (zero || multiple), it returns ok=false +// (matches the conservative server-side resolver in +// cmd/server/extractEdgesFromObs). +func resolvePrefix(idx prefixIndex, hop string) (string, bool) { + h := strings.ToLower(hop) + candidates := idx[h] + if len(candidates) != 1 { + return "", false + } + return candidates[0], true +} diff --git a/cmd/ingestor/neighbor_builder_test.go b/cmd/ingestor/neighbor_builder_test.go new file mode 100644 index 00000000..65b63be7 --- /dev/null +++ b/cmd/ingestor/neighbor_builder_test.go @@ -0,0 +1,87 @@ +package main + +import ( + "path/filepath" + "testing" +) + +// TestNeighborEdgesBuilderUpsertsFromObservations enforces issue +// #1287 Option 4: the INGESTOR builds neighbor_edges from raw +// observations/transmissions and persists them. Server is read-only. +// +// Synthesize a tiny DB with one ADVERT observation whose path[0] +// uniquely resolves to a known node, then assert the builder writes +// the expected edge. +func TestNeighborEdgesBuilderUpsertsFromObservations(t *testing.T) { + dir := t.TempDir() + dbPath := filepath.Join(dir, "build.db") + + // Open via the ingestor's normal opener so applySchema and + // dbschema.Apply both run (the builder requires neighbor_edges + + // observers.iata etc.). + store, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("OpenStore: %v", err) + } + defer store.Close() + + // Seed two nodes whose pubkey prefixes will be used as hops. + if _, err := store.db.Exec( + `INSERT INTO nodes (public_key, name) VALUES (?, ?), (?, ?)`, + "aaaaaaaaaa", "from-node", + "bbbbbbbbbb", "first-hop", + ); err != nil { + t.Fatal(err) + } + + // Seed one observer. + if _, err := store.db.Exec( + `INSERT INTO observers (id, name) VALUES (?, ?)`, + "obs-1", "observer-1", + ); err != nil { + t.Fatal(err) + } + var obsRowid int64 + if err := store.db.QueryRow(`SELECT rowid FROM observers WHERE id = ?`, "obs-1").Scan(&obsRowid); err != nil { + t.Fatal(err) + } + + // Insert one ADVERT transmission with from_pubkey = aaaaa… + res, err := store.db.Exec( + `INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, payload_version, decoded_json, from_pubkey) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + "", "h1", "2026-01-01T00:00:00Z", 0, payloadADVERT, 0, "{}", "aaaaaaaaaa", + ) + if err != nil { + t.Fatal(err) + } + txID, _ := res.LastInsertId() + + // Insert one observation whose path[0] = "bb" (2-hex prefix unique + // to bbbbb… in the nodes table). Expected edge: a↔b. + if _, err := store.db.Exec( + `INSERT INTO observations (transmission_id, observer_idx, path_json, timestamp) VALUES (?, ?, ?, ?)`, + txID, obsRowid, `["bb"]`, int64(1735689600), + ); err != nil { + t.Fatal(err) + } + + n, err := store.buildAndPersistNeighborEdges() + if err != nil { + t.Fatalf("buildAndPersistNeighborEdges: %v", err) + } + if n == 0 { + t.Fatal("expected at least 1 edge upserted, got 0") + } + + var got int + if err := store.db.QueryRow(`SELECT COUNT(*) FROM neighbor_edges WHERE node_a = ? AND node_b = ?`, "aaaaaaaaaa", "bbbbbbbbbb").Scan(&got); err != nil { + t.Fatal(err) + } + if got != 1 { + t.Fatalf("expected the a↔b edge to be persisted; got %d rows", got) + } +} + +// (test ends here) + diff --git a/cmd/migrate/go.mod b/cmd/migrate/go.mod new file mode 100644 index 00000000..b380d96c --- /dev/null +++ b/cmd/migrate/go.mod @@ -0,0 +1,22 @@ +module github.com/corescope/migrate + +go 1.22 + +require ( + github.com/meshcore-analyzer/dbschema v0.0.0 + modernc.org/sqlite v1.34.5 +) + +replace github.com/meshcore-analyzer/dbschema => ../../internal/dbschema + +require ( + github.com/dustin/go-humanize v1.0.1 // indirect + github.com/google/uuid v1.6.0 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/ncruces/go-strftime v0.1.9 // indirect + github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect + golang.org/x/sys v0.22.0 // indirect + modernc.org/libc v1.55.3 // indirect + modernc.org/mathutil v1.6.0 // indirect + modernc.org/memory v1.8.0 // indirect +) diff --git a/cmd/migrate/go.sum b/cmd/migrate/go.sum new file mode 100644 index 00000000..5424fe41 --- /dev/null +++ b/cmd/migrate/go.sum @@ -0,0 +1,43 @@ +github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/google/pprof v0.0.0-20240409012703-83162a5b38cd h1:gbpYu9NMq8jhDVbvlGkMFWCjLFlqqEZjEmObmhUy6Vo= +github.com/google/pprof v0.0.0-20240409012703-83162a5b38cd/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/ncruces/go-strftime v0.1.9 h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdhx/f4= +github.com/ncruces/go-strftime v0.1.9/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= +golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic= +golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= +golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/tools v0.19.0 h1:tfGCXNR1OsFG+sVdLAitlpjAvD/I6dHDKnYrpEZUHkw= +golang.org/x/tools v0.19.0/go.mod h1:qoJWxmGSIBmAeriMx19ogtrEPrGtDbPK634QFIcLAhc= +modernc.org/cc/v4 v4.21.4 h1:3Be/Rdo1fpr8GrQ7IVw9OHtplU4gWbb+wNgeoBMmGLQ= +modernc.org/cc/v4 v4.21.4/go.mod h1:HM7VJTZbUCR3rV8EYBi9wxnJ0ZBRiGE5OeGXNA0IsLQ= +modernc.org/ccgo/v4 v4.19.2 h1:lwQZgvboKD0jBwdaeVCTouxhxAyN6iawF3STraAal8Y= +modernc.org/ccgo/v4 v4.19.2/go.mod h1:ysS3mxiMV38XGRTTcgo0DQTeTmAO4oCmJl1nX9VFI3s= +modernc.org/fileutil v1.3.0 h1:gQ5SIzK3H9kdfai/5x41oQiKValumqNTDXMvKo62HvE= +modernc.org/fileutil v1.3.0/go.mod h1:XatxS8fZi3pS8/hKG2GH/ArUogfxjpEKs3Ku3aK4JyQ= +modernc.org/gc/v2 v2.4.1 h1:9cNzOqPyMJBvrUipmynX0ZohMhcxPtMccYgGOJdOiBw= +modernc.org/gc/v2 v2.4.1/go.mod h1:wzN5dK1AzVGoH6XOzc3YZ+ey/jPgYHLuVckd62P0GYU= +modernc.org/libc v1.55.3 h1:AzcW1mhlPNrRtjS5sS+eW2ISCgSOLLNyFzRh/V3Qj/U= +modernc.org/libc v1.55.3/go.mod h1:qFXepLhz+JjFThQ4kzwzOjA/y/artDeg+pcYnY+Q83w= +modernc.org/mathutil v1.6.0 h1:fRe9+AmYlaej+64JsEEhoWuAYBkOtQiMEU7n/XgfYi4= +modernc.org/mathutil v1.6.0/go.mod h1:Ui5Q9q1TR2gFm0AQRqQUaBWFLAhQpCwNcuhBOSedWPo= +modernc.org/memory v1.8.0 h1:IqGTL6eFMaDZZhEWwcREgeMXYwmW83LYW8cROZYkg+E= +modernc.org/memory v1.8.0/go.mod h1:XPZ936zp5OMKGWPqbD3JShgd/ZoQ7899TUuQqxY+peU= +modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4= +modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0= +modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc= +modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss= +modernc.org/sqlite v1.34.5 h1:Bb6SR13/fjp15jt70CL4f18JIN7p7dnMExd+UFnF15g= +modernc.org/sqlite v1.34.5/go.mod h1:YLuNmX9NKs8wRNK2ko1LW1NGYcc9FkBO69JOt1AR9JE= +modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA= +modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0= +modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= +modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM= diff --git a/cmd/migrate/main.go b/cmd/migrate/main.go new file mode 100644 index 00000000..c10f75a1 --- /dev/null +++ b/cmd/migrate/main.go @@ -0,0 +1,55 @@ +// Command migrate runs all dbschema migrations against a SQLite +// CoreScope database and exits. Used by CI / one-shot tooling to bring +// an unmigrated fixture (or a fresh DB) up to the schema shape the +// read-only server (cmd/server) requires via dbschema.AssertReady. +// +// In production the ingestor (cmd/ingestor) runs dbschema.Apply at +// startup before subscribing to MQTT — this binary exists so CI's E2E +// job can migrate the e2e-fixture.db without booting the full ingestor +// (which needs MQTT brokers). +// +// Usage: +// +// migrate -db path/to/file.db +package main + +import ( + "database/sql" + "flag" + "log" + + "github.com/meshcore-analyzer/dbschema" + _ "modernc.org/sqlite" +) + +func main() { + dbPath := flag.String("db", "", "path to SQLite database to migrate (required)") + flag.Parse() + + if *dbPath == "" { + log.Fatalf("[migrate] -db is required") + } + + log.SetFlags(log.LstdFlags | log.Lmsgprefix) + log.SetPrefix("[migrate] ") + + db, err := sql.Open("sqlite", *dbPath) + if err != nil { + log.Fatalf("open %s: %v", *dbPath, err) + } + defer db.Close() + + if err := db.Ping(); err != nil { + log.Fatalf("ping %s: %v", *dbPath, err) + } + + if err := dbschema.Apply(db, log.Printf); err != nil { + log.Fatalf("dbschema.Apply: %v", err) + } + + if err := dbschema.AssertReady(db); err != nil { + log.Fatalf("dbschema.AssertReady after Apply: %v (this is a bug — Apply did not produce a ready schema)", err) + } + + log.Printf("OK: %s is migrated and ready", *dbPath) +} diff --git a/cmd/migrate/main_test.go b/cmd/migrate/main_test.go new file mode 100644 index 00000000..63956fbc --- /dev/null +++ b/cmd/migrate/main_test.go @@ -0,0 +1,84 @@ +// Test that the migrate binary brings the e2e fixture DB up to the +// shape required by cmd/server's dbschema.AssertReady. Regression test +// for PR #1289 / fix for the CI "Server failed to start within 30s" +// failure: AssertReady fired against the unmigrated fixture and the +// server fatal-logged before opening its HTTP listener. +package main + +import ( + "database/sql" + "io" + "os" + "path/filepath" + "testing" + + "github.com/meshcore-analyzer/dbschema" + _ "modernc.org/sqlite" +) + +// fixtureCandidates lists possible locations of the committed e2e +// fixture DB relative to this test's package directory. We resolve +// against runtime cwd which is cmd/migrate when `go test` runs. +var fixtureCandidates = []string{ + "../../test-fixtures/e2e-fixture.db", +} + +func locateFixture(t *testing.T) string { + t.Helper() + for _, p := range fixtureCandidates { + if _, err := os.Stat(p); err == nil { + abs, _ := filepath.Abs(p) + return abs + } + } + t.Skipf("e2e fixture not found (looked in: %v)", fixtureCandidates) + return "" +} + +func copyFile(t *testing.T, src, dst string) { + t.Helper() + in, err := os.Open(src) + if err != nil { + t.Fatalf("open src: %v", err) + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + t.Fatalf("create dst: %v", err) + } + defer out.Close() + if _, err := io.Copy(out, in); err != nil { + t.Fatalf("copy: %v", err) + } +} + +// TestMigrateBringsFixtureToReady is the gate test for the CI bug. +// Before the fix landed, AssertReady against the committed fixture +// returned an error ("missing: inactive_nodes.foreign_advert" etc.). +// After Apply(), AssertReady must return nil. +func TestMigrateBringsFixtureToReady(t *testing.T) { + src := locateFixture(t) + dst := filepath.Join(t.TempDir(), "fixture-copy.db") + copyFile(t, src, dst) + + db, err := sql.Open("sqlite", dst) + if err != nil { + t.Fatalf("open: %v", err) + } + defer db.Close() + + // Sanity: the committed fixture is missing at least one expected + // migration column. If this stops being true, either someone + // pre-migrated the fixture (and this test no longer protects #1289) + // or AssertReady's required set changed. + if err := dbschema.AssertReady(db); err == nil { + t.Logf("note: fixture already passes AssertReady; skipping pre-condition assertion") + } + + if err := dbschema.Apply(db, t.Logf); err != nil { + t.Fatalf("Apply: %v", err) + } + if err := dbschema.AssertReady(db); err != nil { + t.Fatalf("AssertReady after Apply: %v", err) + } +} diff --git a/cmd/server/backfill_async_test.go b/cmd/server/backfill_async_test.go deleted file mode 100644 index b3df8446..00000000 --- a/cmd/server/backfill_async_test.go +++ /dev/null @@ -1,132 +0,0 @@ -package main - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/gorilla/mux" -) - -// TestBackfillAsyncChunked verifies that backfillResolvedPathsAsync processes -// observations in chunks, yields between batches, and sets the completion flag. -func TestBackfillAsyncChunked(t *testing.T) { - store := &PacketStore{ - packets: make([]*StoreTx, 0), - byHash: make(map[string]*StoreTx), - byTxID: make(map[int]*StoreTx), - byObsID: make(map[int]*StoreObs), - } - - // No pending observations → should complete immediately. - backfillResolvedPathsAsync(store, "", 100, time.Millisecond, 24) - if !store.backfillComplete.Load() { - t.Fatal("expected backfillComplete to be true with empty store") - } -} - -// TestBackfillStatusHeader verifies the X-CoreScope-Status header is set correctly. -func TestBackfillStatusHeader(t *testing.T) { - store := &PacketStore{ - packets: make([]*StoreTx, 0), - byHash: make(map[string]*StoreTx), - byTxID: make(map[int]*StoreTx), - byObsID: make(map[int]*StoreObs), - } - - srv := &Server{store: store} - - handler := srv.backfillStatusMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(200) - })) - - // Before backfill completes → backfilling - req := httptest.NewRequest("GET", "/api/stats", nil) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if got := rec.Header().Get("X-CoreScope-Status"); got != "backfilling" { - t.Fatalf("expected 'backfilling', got %q", got) - } - - // After backfill completes → ready - store.backfillComplete.Store(true) - rec = httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if got := rec.Header().Get("X-CoreScope-Status"); got != "ready" { - t.Fatalf("expected 'ready', got %q", got) - } -} - -// TestStatsBackfillFields verifies /api/stats includes backfill fields. -func TestStatsBackfillFields(t *testing.T) { - db := setupTestDBv2(t) - defer db.Close() - seedV2Data(t, db) - - store := &PacketStore{ - db: db, - packets: make([]*StoreTx, 0), - byHash: make(map[string]*StoreTx), - byTxID: make(map[int]*StoreTx), - byObsID: make(map[int]*StoreObs), - loaded: true, - } - - cfg := &Config{Port: 0} - hub := NewHub() - srv := NewServer(db, cfg, hub) - srv.store = store - - router := mux.NewRouter() - srv.RegisterRoutes(router) - - // While backfilling - req := httptest.NewRequest("GET", "/api/stats", nil) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - var resp map[string]interface{} - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("failed to parse stats response: %v", err) - } - - if backfilling, ok := resp["backfilling"]; !ok { - t.Fatal("missing 'backfilling' field in stats response") - } else if backfilling != true { - t.Fatalf("expected backfilling=true, got %v", backfilling) - } - - if _, ok := resp["backfillProgress"]; !ok { - t.Fatal("missing 'backfillProgress' field in stats response") - } - - // Check header - if got := rec.Header().Get("X-CoreScope-Status"); got != "backfilling" { - t.Fatalf("expected X-CoreScope-Status=backfilling, got %q", got) - } - - // After backfill completes - store.backfillComplete.Store(true) - // Invalidate stats cache - srv.statsMu.Lock() - srv.statsCache = nil - srv.statsMu.Unlock() - - rec = httptest.NewRecorder() - router.ServeHTTP(rec, req) - - resp = nil - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("failed to parse stats response: %v", err) - } - - if backfilling, ok := resp["backfilling"]; !ok || backfilling != false { - t.Fatalf("expected backfilling=false after completion, got %v", backfilling) - } - - if got := rec.Header().Get("X-CoreScope-Status"); got != "ready" { - t.Fatalf("expected X-CoreScope-Status=ready, got %q", got) - } -} diff --git a/cmd/server/config_knobs_test.go b/cmd/server/config_knobs_test.go index 9a3cd4fa..2d3b2b7a 100644 --- a/cmd/server/config_knobs_test.go +++ b/cmd/server/config_knobs_test.go @@ -1,12 +1,8 @@ package main import ( - "database/sql" - "path/filepath" "testing" "time" - - _ "modernc.org/sqlite" ) func TestBackfillHoursDefault(t *testing.T) { @@ -72,106 +68,3 @@ func TestGraphPruneOlderThan(t *testing.T) { } } -func TestPruneNeighborEdgesDB(t *testing.T) { - tmpDir := t.TempDir() - dbPath := filepath.Join(tmpDir, "test.db") - db, err := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - if err != nil { - t.Fatal(err) - } - defer db.Close() - - _, err = db.Exec(`CREATE TABLE neighbor_edges ( - node_a TEXT NOT NULL, - node_b TEXT NOT NULL, - count INTEGER DEFAULT 1, - last_seen TEXT, - PRIMARY KEY (node_a, node_b) - )`) - if err != nil { - t.Fatal(err) - } - - now := time.Now().UTC() - old := now.Add(-60 * 24 * time.Hour) - - db.Exec("INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) VALUES (?, ?, 5, ?)", - "aaa", "bbb", now.Format(time.RFC3339)) - db.Exec("INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) VALUES (?, ?, 3, ?)", - "ccc", "ddd", old.Format(time.RFC3339)) - - g := NewNeighborGraph() - g.upsertEdge("aaa", "bbb", "bb", "obs1", nil, now) - g.upsertEdge("ccc", "ddd", "dd", "obs1", nil, old) - - pruned, err := PruneNeighborEdges(dbPath, g, 30) - if err != nil { - t.Fatal(err) - } - if pruned != 1 { - t.Errorf("PruneNeighborEdges pruned %d DB rows, want 1", pruned) - } - - var count int - db.QueryRow("SELECT COUNT(*) FROM neighbor_edges").Scan(&count) - if count != 1 { - t.Errorf("expected 1 row in DB after prune, got %d", count) - } - - if len(g.AllEdges()) != 1 { - t.Errorf("expected 1 in-memory edge after prune, got %d", len(g.AllEdges())) - } -} - -func TestBackfillRespectsHourWindow(t *testing.T) { - store := &PacketStore{} - - now := time.Now().UTC() - oldTime := now.Add(-48 * time.Hour).Format(time.RFC3339Nano) - newTime := now.Add(-30 * time.Minute).Format(time.RFC3339Nano) - - store.packets = []*StoreTx{ - { - ID: 1, - Hash: "old-hash", - FirstSeen: oldTime, - Observations: []*StoreObs{ - {ID: 1, PathJSON: `["abc"]`}, - }, - }, - { - ID: 2, - Hash: "new-hash", - FirstSeen: newTime, - Observations: []*StoreObs{ - {ID: 2, PathJSON: `["def"]`}, - }, - }, - } - - // With a 1-hour window, only the new tx should be processed. - // backfillResolvedPathsAsync will find no prefix map and finish quickly, - // but we can verify the pending count reflects the window. - go backfillResolvedPathsAsync(store, "", 100, time.Millisecond, 1) - - // Wait for completion - for i := 0; i < 100; i++ { - if store.backfillComplete.Load() { - break - } - time.Sleep(10 * time.Millisecond) - } - - if !store.backfillComplete.Load() { - t.Fatal("backfill did not complete") - } - - // With no prefix map, total should be 0 (early exit) or just the new one - // The function exits early when pm == nil, so backfillTotal stays at 0 - // if there were pending items but no pm. Let's verify it didn't process - // the old one by checking total <= 1. - total := store.backfillTotal.Load() - if total > 1 { - t.Errorf("backfill total = %d, want <= 1 (old tx should be excluded by hour window)", total) - } -} diff --git a/cmd/server/ensure_indexes.go b/cmd/server/ensure_indexes.go deleted file mode 100644 index 5d4a0fa1..00000000 --- a/cmd/server/ensure_indexes.go +++ /dev/null @@ -1,75 +0,0 @@ -package main - -import ( - "fmt" - "strings" -) - -// ensureServerIndexes creates the indexes that the SQL fallback path in -// QueryPackets / QueryGroupedPackets and the background hot-startup chunk -// loader depend on. Mirrors the indexes the ingestor creates (see -// cmd/ingestor/db.go applySchema). Safe to call on every server start -// because every CREATE INDEX uses IF NOT EXISTS. Needed because DBs -// created by an old server-only build (pre-ingestor) won't have the -// ingestor's indexes, which would cause full table scans on the SQL -// fallback path during hot startup. -func ensureServerIndexes(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return fmt.Errorf("open rw for index ensure: %w", err) - } - stmts := []string{ - `CREATE INDEX IF NOT EXISTS idx_transmissions_first_seen ON transmissions(first_seen)`, - `CREATE INDEX IF NOT EXISTS idx_transmissions_hash ON transmissions(hash)`, - `CREATE INDEX IF NOT EXISTS idx_transmissions_payload_type ON transmissions(payload_type)`, - // PR #1187 r3: commit 63cc1bc3 restored the RFC3339 since/until path - // to a SELECT … FROM observations WHERE timestamp >= ? subquery in - // buildTransmissionWhere. Without these indexes the subquery - // full-scans observations on legacy server-only DBs (the ingestor - // already creates them; see cmd/ingestor/db.go applySchema). - `CREATE INDEX IF NOT EXISTS idx_observations_timestamp ON observations(timestamp)`, - `CREATE INDEX IF NOT EXISTS idx_observations_transmission_id ON observations(transmission_id)`, - } - for _, s := range stmts { - if _, err := rw.Exec(s); err != nil { - return fmt.Errorf("ensure index %q: %w", s, err) - } - } - - // observer_idx column exists in v3 schema only; observer_id is the - // v2 equivalent. Probe the schema and create the matching index. - rows, err := rw.Query(`PRAGMA table_info(observations)`) - if err != nil { - return fmt.Errorf("pragma table_info(observations): %w", err) - } - var hasObserverIdx, hasObserverID bool - for rows.Next() { - var cid int - var name, ctype string - var notnull, pk int - var dflt interface{} - if err := rows.Scan(&cid, &name, &ctype, ¬null, &dflt, &pk); err != nil { - rows.Close() - return fmt.Errorf("scan table_info: %w", err) - } - switch strings.ToLower(name) { - case "observer_idx": - hasObserverIdx = true - case "observer_id": - hasObserverID = true - } - } - rows.Close() - - if hasObserverIdx { - if _, err := rw.Exec(`CREATE INDEX IF NOT EXISTS idx_observations_observer_idx ON observations(observer_idx)`); err != nil { - return fmt.Errorf("ensure idx_observations_observer_idx: %w", err) - } - } - if hasObserverID { - if _, err := rw.Exec(`CREATE INDEX IF NOT EXISTS idx_observations_observer_id ON observations(observer_id)`); err != nil { - return fmt.Errorf("ensure idx_observations_observer_id: %w", err) - } - } - return nil -} diff --git a/cmd/server/ensure_indexes_test.go b/cmd/server/ensure_indexes_test.go deleted file mode 100644 index b1fab55b..00000000 --- a/cmd/server/ensure_indexes_test.go +++ /dev/null @@ -1,75 +0,0 @@ -package main - -import ( - "database/sql" - "path/filepath" - "testing" - - _ "modernc.org/sqlite" -) - -// TestEnsureServerIndexes_CreatesObservationsIndexes guards against -// regression of PR #1187 r3 MUST-FIX 2: legacy server-only DBs that lack -// the ingestor-created observation indexes used to full-scan the -// `SELECT ... FROM observations WHERE timestamp >= ?` subquery added to -// buildTransmissionWhere by 63cc1bc3. ensureServerIndexes must create -// idx_observations_timestamp (and the join companions) so the hot-startup -// chunk loader and RFC3339 since/until path don't full-scan observations. -func TestEnsureServerIndexes_CreatesObservationsIndexes(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "schema_only.db") - - conn, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatalf("open: %v", err) - } - - // Minimal legacy server-only schema: tables present, no extra indexes. - stmts := []string{ - `CREATE TABLE transmissions (id INTEGER PRIMARY KEY, raw_hex TEXT, hash TEXT, first_seen TEXT, route_type INTEGER, payload_type INTEGER, payload_version INTEGER, decoded_json TEXT)`, - // v3 schema (observer_idx) — matches the ingestor-created shape - // and the path that 63cc1bc3 / hot-startup loadChunk traverse. - `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER, observer_idx INTEGER, direction TEXT, snr REAL, rssi REAL, score INTEGER, path_json TEXT, timestamp TEXT, raw_hex TEXT)`, - `CREATE TABLE observers (rowid INTEGER PRIMARY KEY, id TEXT, name TEXT)`, - `CREATE TABLE nodes (pubkey TEXT PRIMARY KEY, name TEXT, role TEXT, lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, frequency REAL)`, - `CREATE TABLE schema_version (version INTEGER)`, - `INSERT INTO schema_version (version) VALUES (1)`, - } - for _, s := range stmts { - if _, err := conn.Exec(s); err != nil { - t.Fatalf("setup %q: %v", s, err) - } - } - conn.Close() - - if err := ensureServerIndexes(dbPath); err != nil { - t.Fatalf("ensureServerIndexes: %v", err) - } - - // Reopen and query sqlite_master for the indexes we expect. - conn2, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatalf("reopen: %v", err) - } - defer conn2.Close() - - required := []string{ - "idx_transmissions_first_seen", - "idx_transmissions_hash", - "idx_transmissions_payload_type", - "idx_observations_timestamp", - "idx_observations_transmission_id", - "idx_observations_observer_idx", - } - for _, name := range required { - var found string - err := conn2.QueryRow(`SELECT name FROM sqlite_master WHERE type='index' AND name=?`, name).Scan(&found) - if err != nil { - t.Errorf("index %s not created (err=%v) — ensureServerIndexes must create it to avoid full scans on the SQL fallback path", name, err) - continue - } - if found != name { - t.Errorf("index lookup mismatch: want %s got %s", name, found) - } - } -} diff --git a/cmd/server/from_pubkey_attribution_test.go b/cmd/server/from_pubkey_attribution_test.go deleted file mode 100644 index 9ef5c320..00000000 --- a/cmd/server/from_pubkey_attribution_test.go +++ /dev/null @@ -1,434 +0,0 @@ -package main - -// Tests for issue #1143: pubkey attribution must use exact-match on a -// dedicated `from_pubkey` column, not `decoded_json LIKE '%pubkey%'`. -// -// These tests demonstrate the structural holes documented in #1143: -// Hole 1: name-LIKE fallback surfaces same-name nodes -// Hole 2a: an attacker can name themselves with someone else's pubkey -// and get their transmissions attributed to the victim -// Hole 2b: any 64-char hex substring inside decoded_json (path elements, -// channel names, message bodies) produces false positives - -import ( - "database/sql" - "fmt" - "strings" - "testing" - "time" - - _ "modernc.org/sqlite" -) - -const ( - pkVictim = "f7181c468dfe7c55aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - pkAttacker = "deadbeefdeadbeefcccccccccccccccccccccccccccccccccccccccccccccccc" - pkOther = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" -) - -// seedAttribution inserts the standard adversarial fixture used by the -// issue #1143 tests. It returns the victim pubkey for convenience. -func seedAttribution(t *testing.T, db *DB) string { - t.Helper() - now := time.Now().UTC().Format(time.RFC3339) - - // (1) Legitimate ADVERT from the victim. - mustExec(t, db, `INSERT INTO transmissions - (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, from_pubkey) - VALUES ('AA','h_victim_advert',?,1,4, - '{"type":"ADVERT","pubKey":"`+pkVictim+`","name":"VictimNode"}', - ?)`, now, pkVictim) - - // (2) Hole 1: a different node sharing the *display name* "VictimNode". - mustExec(t, db, `INSERT INTO transmissions - (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, from_pubkey) - VALUES ('BB','h_namespoof_advert',?,1,4, - '{"type":"ADVERT","pubKey":"`+pkOther+`","name":"VictimNode"}', - ?)`, now, pkOther) - - // (3) Hole 2a: malicious node whose *name* is the victim's pubkey. - // decoded_json contains pkVictim as a substring (in the name field), - // but the actual originator is pkAttacker. - mustExec(t, db, `INSERT INTO transmissions - (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, from_pubkey) - VALUES ('CC','h_spoof_advert',?,1,4, - '{"type":"ADVERT","pubKey":"`+pkAttacker+`","name":"`+pkVictim+`"}', - ?)`, now, pkAttacker) - - // (4) Hole 2b: free-text packet (e.g. channel message) whose body - // coincidentally contains the victim's pubkey as a substring. - // Real originator is pkAttacker; from_pubkey reflects that. - mustExec(t, db, `INSERT INTO transmissions - (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, from_pubkey) - VALUES ('DD','h_freetext_msg',?,1,5, - '{"type":"GRP_TXT","text":"hello `+pkVictim+` how are you"}', - ?)`, now, pkAttacker) - - return pkVictim -} - -func mustExec(t *testing.T, db *DB, q string, args ...interface{}) { - t.Helper() - if _, err := db.conn.Exec(q, args...); err != nil { - t.Fatalf("exec failed: %v\nquery: %s", err, q) - } -} - -func hashesOf(rows []map[string]interface{}) []string { - out := make([]string, 0, len(rows)) - for _, r := range rows { - if h, ok := r["hash"].(string); ok { - out = append(out, h) - } - } - return out -} - -func TestRecentTransmissions_Hole1_SameNameDifferentPubkey(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - victim := seedAttribution(t, db) - - got, err := db.GetRecentTransmissionsForNode(victim, 20) - if err != nil { - t.Fatal(err) - } - - hashes := hashesOf(got) - for _, h := range hashes { - if h == "h_namespoof_advert" { - t.Fatalf("Hole 1: same-name node was attributed to the victim. got hashes=%v", hashes) - } - } -} - -func TestRecentTransmissions_Hole2a_PubkeyAsNameSpoof(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - victim := seedAttribution(t, db) - - got, err := db.GetRecentTransmissionsForNode(victim, 20) - if err != nil { - t.Fatal(err) - } - - hashes := hashesOf(got) - for _, h := range hashes { - if h == "h_spoof_advert" { - t.Fatalf("Hole 2a: attacker who named themselves with victim's pubkey "+ - "was attributed to the victim. got hashes=%v", hashes) - } - } -} - -func TestRecentTransmissions_Hole2b_FreeTextHexFalsePositive(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - victim := seedAttribution(t, db) - - got, err := db.GetRecentTransmissionsForNode(victim, 20) - if err != nil { - t.Fatal(err) - } - - hashes := hashesOf(got) - for _, h := range hashes { - if h == "h_freetext_msg" { - t.Fatalf("Hole 2b: free-text containing the victim's pubkey as a "+ - "substring produced a false positive. got hashes=%v", hashes) - } - } -} - -func TestRecentTransmissions_LegitimateAdvertReturned(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - victim := seedAttribution(t, db) - - got, err := db.GetRecentTransmissionsForNode(victim, 20) - if err != nil { - t.Fatal(err) - } - - hashes := hashesOf(got) - found := false - for _, h := range hashes { - if h == "h_victim_advert" { - found = true - break - } - } - if !found { - t.Fatalf("expected legitimate victim advert (h_victim_advert) in result, got %v", hashes) - } -} - -// --- Multi-pubkey OR query (#1143 — db.go:1785) --- - -func TestQueryMultiNodePackets_ExactMatchOnly(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - seedAttribution(t, db) - - // Query the victim's pubkey via the multi-node API. The malicious - // "name = victim pubkey" row and the free-text row must NOT show up. - res, err := db.QueryMultiNodePackets([]string{pkVictim}, 50, 0, "DESC", "", "") - if err != nil { - t.Fatal(err) - } - hashes := hashesOf(res.Packets) - for _, bad := range []string{"h_spoof_advert", "h_freetext_msg", "h_namespoof_advert"} { - for _, h := range hashes { - if h == bad { - t.Fatalf("QueryMultiNodePackets returned spurious match %q (pubkey %s as substring); hashes=%v", - bad, pkVictim, hashes) - } - } - } - // The legitimate one must still be present. - if !contains(hashes, "h_victim_advert") { - t.Fatalf("expected h_victim_advert in QueryMultiNodePackets result, got %v", hashes) - } -} - -func contains(haystack []string, needle string) bool { - for _, s := range haystack { - if s == needle { - return true - } - } - return false -} - -// --- Index sanity check (#1143 perf): verify EXPLAIN QUERY PLAN uses the -// new index, not a SCAN. --- - -func TestFromPubkeyIndexUsed(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - mustExec(t, db, `CREATE INDEX IF NOT EXISTS idx_transmissions_from_pubkey ON transmissions(from_pubkey)`) - - rows, err := db.conn.Query( - `EXPLAIN QUERY PLAN SELECT id FROM transmissions WHERE from_pubkey = ?`, - pkVictim) - if err != nil { - t.Fatal(err) - } - defer rows.Close() - plan := "" - for rows.Next() { - var id, parent, notused int - var detail string - if err := rows.Scan(&id, &parent, ¬used, &detail); err == nil { - plan += detail + "\n" - } - } - if !strings.Contains(plan, "idx_transmissions_from_pubkey") { - t.Fatalf("expected EXPLAIN QUERY PLAN to use idx_transmissions_from_pubkey, got:\n%s", plan) - } -} - -// TestFromPubkeyIndexUsedForInClause verifies the index is used for the -// IN (?, ?, ...) query path used by QueryMultiNodePackets (db.go ~1787). -// Coverage extension — the equality path is covered above; this asserts -// the multi-node path doesn't silently regress to a full scan when the -// planner can't use the index for set membership. -func TestFromPubkeyIndexUsedForInClause(t *testing.T) { - db := setupTestDB(t) - defer db.Close() - mustExec(t, db, `CREATE INDEX IF NOT EXISTS idx_transmissions_from_pubkey ON transmissions(from_pubkey)`) - - rows, err := db.conn.Query( - `EXPLAIN QUERY PLAN SELECT id FROM transmissions WHERE from_pubkey IN (?, ?)`, - pkVictim, pkOther) - if err != nil { - t.Fatal(err) - } - defer rows.Close() - plan := "" - for rows.Next() { - var id, parent, notused int - var detail string - if err := rows.Scan(&id, &parent, ¬used, &detail); err == nil { - plan += detail + "\n" - } - } - if !strings.Contains(plan, "idx_transmissions_from_pubkey") { - t.Fatalf("expected EXPLAIN QUERY PLAN for IN(...) to use idx_transmissions_from_pubkey, got:\n%s", plan) - } -} - -// --- Migration / backfill --- - -func TestBackfillFromPubkey_AdvertRowsPopulated(t *testing.T) { - dir := t.TempDir() - dbPath := dir + "/test.db" - - // Create a legacy-style DB: transmissions table WITHOUT from_pubkey, - // then run ensureFromPubkeyColumn to ALTER it in. - rw, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - if _, err := rw.Exec(`CREATE TABLE transmissions ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - raw_hex TEXT, hash TEXT UNIQUE, first_seen TEXT, - route_type INTEGER, payload_type INTEGER, payload_version INTEGER, - decoded_json TEXT, created_at TEXT - )`); err != nil { - t.Fatal(err) - } - // Two ADVERTs (different pubkeys) and a non-ADVERT. - if _, err := rw.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, payload_type, decoded_json) VALUES - ('AA','m1','2026-01-01T00:00:00Z',4,'{"type":"ADVERT","pubKey":"`+pkVictim+`","name":"V"}'), - ('BB','m2','2026-01-01T00:00:00Z',4,'{"type":"ADVERT","pubKey":"`+pkOther+`","name":"O"}'), - ('CC','m3','2026-01-01T00:00:00Z',5,'{"type":"GRP_TXT","text":"hi"}')`); err != nil { - t.Fatal(err) - } - rw.Close() - - if err := ensureFromPubkeyColumn(dbPath); err != nil { - t.Fatalf("ensureFromPubkeyColumn: %v", err) - } - - // Run synchronously by calling the function directly. - backfillFromPubkeyAsync(dbPath, 100, 0) - - // Verify backfill populated the ADVERT rows. - rw2, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - defer rw2.Close() - rows, err := rw2.Query("SELECT hash, from_pubkey FROM transmissions ORDER BY hash") - if err != nil { - t.Fatal(err) - } - defer rows.Close() - got := map[string]string{} - for rows.Next() { - var h string - var pk sql.NullString - if err := rows.Scan(&h, &pk); err != nil { - t.Fatal(err) - } - got[h] = pk.String - } - if got["m1"] != pkVictim { - t.Errorf("m1 from_pubkey = %q, want %q", got["m1"], pkVictim) - } - if got["m2"] != pkOther { - t.Errorf("m2 from_pubkey = %q, want %q", got["m2"], pkOther) - } - // Non-ADVERT row was not in the backfill scope; from_pubkey stays NULL. - if got["m3"] != "" { - t.Errorf("m3 from_pubkey = %q, want empty (NULL)", got["m3"]) - } -} - -// TestBackfillFromPubkey_DoesNotBlockBoot exercises the async contract: -// main.go (cmd/server/main.go) calls startFromPubkeyBackfill, which is the -// SAME entry point used at production startup. The wrapper must dispatch -// the backfill in a goroutine; if anyone removes the `go` keyword inside -// startFromPubkeyBackfill, this test fails because the call no longer -// returns within the 50ms boot dispatch budget. The test does NOT use `go` -// itself — that would test only the test's own scheduler, not the -// production code path (cycle-3 M1c). -// -// DO NOT t.Parallel — uses package-global atomics -// (fromPubkeyBackfillTotal/Processed/Done). Concurrent tests would clobber -// the resets (cycle-3 m1c). -func TestBackfillFromPubkey_DoesNotBlockBoot(t *testing.T) { - dir := t.TempDir() - dbPath := dir + "/async_boot.db" - - rw, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - if _, err := rw.Exec(`CREATE TABLE transmissions ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - raw_hex TEXT, hash TEXT UNIQUE, first_seen TEXT, - route_type INTEGER, payload_type INTEGER, payload_version INTEGER, - decoded_json TEXT, created_at TEXT - )`); err != nil { - t.Fatal(err) - } - // Insert N=1000 legacy ADVERT rows. With chunkSize=100 + yield=100ms - // between chunks, sync would be ~900ms; we assert dispatch is <50ms. - tx, err := rw.Begin() - if err != nil { - t.Fatal(err) - } - stmt, err := tx.Prepare(`INSERT INTO transmissions - (raw_hex, hash, first_seen, payload_type, decoded_json) VALUES (?, ?, ?, 4, ?)`) - if err != nil { - t.Fatal(err) - } - const N = 1000 - for i := 0; i < N; i++ { - hash := fmt.Sprintf("h_async_boot_%d", i) - dj := fmt.Sprintf(`{"type":"ADVERT","pubKey":"%s","name":"N%d"}`, pkVictim, i) - if _, err := stmt.Exec("AA", hash, "2026-01-01T00:00:00Z", dj); err != nil { - t.Fatal(err) - } - } - stmt.Close() - if err := tx.Commit(); err != nil { - t.Fatal(err) - } - rw.Close() - - if err := ensureFromPubkeyColumn(dbPath); err != nil { - t.Fatalf("ensureFromPubkeyColumn: %v", err) - } - - // Reset all backfill state — other tests may have set it. - fromPubkeyBackfillReset() - defer fromPubkeyBackfillReset() - - // Dispatch via the production wrapper. startFromPubkeyBackfill is the - // same entry point main.go calls at boot; it must launch the backfill - // in a goroutine internally. We deliberately do NOT prefix `go` here — - // if the wrapper is ever made synchronous, the dispatch budget below - // fires first. - t0 := time.Now() - startFromPubkeyBackfill(dbPath, 100, 100*time.Millisecond) - dispatchElapsed := time.Since(t0) - - // (a) Boot-time dispatch budget: must return ~immediately. - if dispatchElapsed > 50*time.Millisecond { - t.Fatalf("backfill dispatch took %v (>50ms): not async — would block boot", dispatchElapsed) - } - - // (b) Eventual completion via the fromPubkeyBackfill snapshot. - deadline := time.Now().Add(30 * time.Second) - for time.Now().Before(deadline) { - if _, _, done := fromPubkeyBackfillSnapshot(); done { - break - } - time.Sleep(50 * time.Millisecond) - } - if _, _, done := fromPubkeyBackfillSnapshot(); !done { - t.Fatalf("backfill never flipped Done within 30s; dispatched=%v", dispatchElapsed) - } - - // (c) Backfill actually populated rows. - rw2, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - defer rw2.Close() - var nullCount int - if err := rw2.QueryRow( - `SELECT COUNT(*) FROM transmissions WHERE payload_type = 4 AND from_pubkey IS NULL`, - ).Scan(&nullCount); err != nil { - t.Fatal(err) - } - if nullCount > 0 { - t.Errorf("backfill left %d ADVERT rows with NULL from_pubkey", nullCount) - } - if _, processed, _ := fromPubkeyBackfillSnapshot(); processed != int64(N) { - t.Errorf("fromPubkeyBackfillProcessed = %d, want %d", processed, N) - } -} diff --git a/cmd/server/from_pubkey_migration.go b/cmd/server/from_pubkey_migration.go index b01cd7eb..ef530005 100644 --- a/cmd/server/from_pubkey_migration.go +++ b/cmd/server/from_pubkey_migration.go @@ -1,261 +1,38 @@ +// Package main: from_pubkey backfill shim (issue #1287). +// +// The actual backfill moved to cmd/ingestor (see +// cmd/ingestor/maintenance.go: BackfillFromPubkey) because the server +// is the read path and may not write to SQLite (#1283/#1287). This +// file retains the snapshot getter so /api/healthz still compiles — +// it always reports done=true with zero counters. Operators monitor +// the ingestor's stats file for true progress. package main -// from_pubkey migration (#1143). -// -// Adds the `transmissions.from_pubkey` column + index, and provides an async -// backfill that populates the column from `decoded_json` for ADVERT packets -// whose `from_pubkey` is still NULL. -// -// Why a column at all: the legacy attribution path used -// `WHERE decoded_json LIKE '%pubkey%'` (and `OR LIKE '%name%'`). This is -// structurally unsound (adversarial spoofing + accidental hex-substring -// false positives + full table scan). The column gives us exact match, -// O(log n) lookups, and an explicit, auditable attribution surface. -// -// Backfill is run async (best-effort) so it cannot block server startup -// even on prod-sized DBs (100K+ transmissions). Queries handle NULL -// gracefully (return empty for that pubkey, same as today's behaviour -// for unknown pubkeys). +import "sync" -import ( - "database/sql" - "encoding/json" - "fmt" - "log" - "sync" - "time" -) - -// ensureFromPubkeyColumn adds the from_pubkey column + index to the -// transmissions table if missing. Safe to call repeatedly. -func ensureFromPubkeyColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - - has, err := tableHasColumn(rw, "transmissions", "from_pubkey") - if err != nil { - return fmt.Errorf("inspect transmissions: %w", err) - } - if !has { - if _, err := rw.Exec("ALTER TABLE transmissions ADD COLUMN from_pubkey TEXT"); err != nil { - return fmt.Errorf("add from_pubkey column: %w", err) - } - log.Println("[store] Added from_pubkey column to transmissions (#1143)") - } - - if _, err := rw.Exec("CREATE INDEX IF NOT EXISTS idx_transmissions_from_pubkey ON transmissions(from_pubkey)"); err != nil { - return fmt.Errorf("create idx_transmissions_from_pubkey: %w", err) - } - return nil -} - -// fromPubkeyBackfillProgress reports backfill state for /api/healthz. -// All three values are read together via fromPubkeyBackfillSnapshot() -// under a single RWMutex so /api/healthz never sees a torn snapshot -// (e.g. done=true with processed decoded_json.pubKey -// - other types -> leave NULL (queries handle NULL gracefully) -// -// chunkSize and yieldDuration are tunable for tests. -func backfillFromPubkeyAsync(dbPath string, chunkSize int, yieldDuration time.Duration) { - defer func() { - if r := recover(); r != nil { - log.Printf("[store] backfillFromPubkeyAsync panic recovered: %v", r) - } - fromPubkeyBackfillMarkDone() - }() - - if chunkSize <= 0 { - chunkSize = 5000 - } - - rw, err := cachedRW(dbPath) - if err != nil { - log.Printf("[store] from_pubkey backfill: open rw error: %v", err) - return - } - - var total int64 - if err := rw.QueryRow( - "SELECT COUNT(*) FROM transmissions WHERE from_pubkey IS NULL AND payload_type = 4", - ).Scan(&total); err != nil { - log.Printf("[store] from_pubkey backfill: count error: %v", err) - return - } - fromPubkeyBackfillSetTotal(total) - if total == 0 { - log.Println("[store] from_pubkey backfill: nothing to do") - return - } - log.Printf("[store] from_pubkey backfill starting: %d ADVERT rows", total) - - updateStmt, err := rw.Prepare("UPDATE transmissions SET from_pubkey = ? WHERE id = ?") - if err != nil { - log.Printf("[store] from_pubkey backfill: prepare update: %v", err) - return - } - defer updateStmt.Close() - - var processed int64 - for { - rows, err := rw.Query( - "SELECT id, decoded_json FROM transmissions WHERE from_pubkey IS NULL AND payload_type = 4 LIMIT ?", - chunkSize) - if err != nil { - log.Printf("[store] from_pubkey backfill: select error: %v", err) - return - } - - type row struct { - id int64 - pk string - } - batch := make([]row, 0, chunkSize) - for rows.Next() { - var id int64 - var dj sql.NullString - if err := rows.Scan(&id, &dj); err != nil { - continue - } - pk := extractPubkeyFromAdvertJSON(dj.String) - batch = append(batch, row{id: id, pk: pk}) - } - rows.Close() - - if len(batch) == 0 { - break - } - - // Apply updates in a single tx for throughput. - tx, err := rw.Begin() - if err != nil { - log.Printf("[store] from_pubkey backfill: begin tx: %v", err) - return - } - txStmt := tx.Stmt(updateStmt) - for _, b := range batch { - // Sentinel convention for transmissions.from_pubkey (#1143, m5): - // NULL — row has not yet been scanned by this backfill. - // "" — scanned, no extractable pubkey (malformed/legacy ADVERT - // decoded_json, or a JSON shape we don't understand). - // hex — scanned, pubkey successfully extracted. - // - // The "" sentinel exists ONLY in this backfill path: it's how we - // avoid the #1119 infinite-rescan loop (the WHERE clause is - // `from_pubkey IS NULL`, so once we mark a row "" it never matches - // again). The ingest write path (cmd/ingestor/db.go ~1289) leaves - // from_pubkey NULL when PubKey is empty; the two states are - // semantically equivalent ("we have no pubkey for this row") and - // all attribution call sites query `from_pubkey = ?` with a real - // pubkey, so neither NULL nor "" matches — no UX divergence. - var val interface{} - if b.pk != "" { - val = b.pk - } else { - val = "" // scanned, no extractable pubkey — see comment above - } - if _, err := txStmt.Exec(val, b.id); err != nil { - // non-fatal; log first failure per chunk and keep going - log.Printf("[store] from_pubkey backfill: update id=%d: %v", b.id, err) - } - } - if err := tx.Commit(); err != nil { - log.Printf("[store] from_pubkey backfill: commit: %v", err) - return - } - processed += int64(len(batch)) - fromPubkeyBackfillSetProcessed(processed) - - if len(batch) < chunkSize { - break - } - if yieldDuration > 0 { - time.Sleep(yieldDuration) - } - } - log.Printf("[store] from_pubkey backfill complete: %d rows processed", processed) -} - -// extractPubkeyFromAdvertJSON parses an ADVERT decoded_json blob and returns -// the pubKey field, or "" if absent/invalid. Lenient: any parse error yields -// the empty string rather than a panic. -func extractPubkeyFromAdvertJSON(s string) string { - if s == "" { - return "" - } - var m map[string]interface{} - if err := json.Unmarshal([]byte(s), &m); err != nil { - return "" - } - if v, ok := m["pubKey"].(string); ok { - return v - } - return "" -} diff --git a/cmd/server/go.mod b/cmd/server/go.mod index ee2b1452..e150a42b 100644 --- a/cmd/server/go.mod +++ b/cmd/server/go.mod @@ -26,6 +26,10 @@ require github.com/meshcore-analyzer/perfio v0.0.0 replace github.com/meshcore-analyzer/perfio => ../../internal/perfio +require github.com/meshcore-analyzer/dbschema v0.0.0 + +replace github.com/meshcore-analyzer/dbschema => ../../internal/dbschema + require ( github.com/dustin/go-humanize v1.0.1 // indirect github.com/google/uuid v1.6.0 // indirect diff --git a/cmd/server/healthz_test.go b/cmd/server/healthz_test.go index 462a7300..dce8e163 100644 --- a/cmd/server/healthz_test.go +++ b/cmd/server/healthz_test.go @@ -2,14 +2,10 @@ package main import ( "encoding/json" - "fmt" "net/http" "net/http/httptest" - "sync" "testing" - "time" ) - func TestHealthzNotReady(t *testing.T) { // Ensure readiness is 0 (not ready) readiness.Store(0) @@ -82,150 +78,3 @@ func TestHealthzAntiTautology(t *testing.T) { } } -// TestHealthzExposesFromPubkeyBackfill verifies the from_pubkey backfill -// progress (#1143, M2) is observable via /api/healthz. The atomics are -// updated by backfillFromPubkeyAsync; without exposure here they were dead -// code. Asserts the response includes a from_pubkey_backfill object with -// total/processed/done fields. -func TestHealthzExposesFromPubkeyBackfill(t *testing.T) { - readiness.Store(1) - defer readiness.Store(0) - - // Set known values so we can assert wiring (not just presence). - fromPubkeyBackfillReset() - fromPubkeyBackfillSetTotal(7) - fromPubkeyBackfillSetProcessed(3) - defer fromPubkeyBackfillReset() - - srv := &Server{store: &PacketStore{}} - req := httptest.NewRequest("GET", "/api/healthz", nil) - w := httptest.NewRecorder() - srv.handleHealthz(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d", w.Code) - } - var resp map[string]interface{} - if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { - t.Fatalf("invalid JSON: %v", err) - } - bf, ok := resp["from_pubkey_backfill"].(map[string]interface{}) - if !ok { - t.Fatalf("missing from_pubkey_backfill object in healthz response: %v", resp) - } - if got, want := bf["total"], float64(7); got != want { - t.Errorf("from_pubkey_backfill.total = %v, want %v", got, want) - } - if got, want := bf["processed"], float64(3); got != want { - t.Errorf("from_pubkey_backfill.processed = %v, want %v", got, want) - } - if got, want := bf["done"], false; got != want { - t.Errorf("from_pubkey_backfill.done = %v, want %v", got, want) - } -} - -// TestHealthzFromPubkeyBackfillConsistentSnapshot exercises cycle-3 m2c: -// the handler used to read three independent atomics (Total/Processed/Done) -// in sequence, so a backfill update interleaved between reads could yield -// an inconsistent snapshot (e.g. done=true with processedtotal when total is updated last). This test races concurrent -// progress updates against many healthz reads and asserts every snapshot -// satisfies the invariants: -// -// processed <= total -// if done: processed == total (or both 0 — nothing to do) -// -// With the pre-fix code (separate atomic.Load calls), this fires within -// a few hundred iterations on a multi-core box. With the RWMutex-guarded -// snapshot, it never fires. -func TestHealthzFromPubkeyBackfillConsistentSnapshot(t *testing.T) { - readiness.Store(1) - defer readiness.Store(0) - defer fromPubkeyBackfillReset() - - srv := &Server{store: &PacketStore{}} - - stop := make(chan struct{}) - var writerWg sync.WaitGroup - var readerWg sync.WaitGroup - - // Writer: simulates the backfill loop — sets total, then increments - // processed in lock-step, occasionally finishing (done=true with - // processed==total). Each "tick" mutates all three values. - writerWg.Add(1) - go func() { - defer writerWg.Done() - for { - select { - case <-stop: - return - default: - } - fromPubkeyBackfillSetTotal(100) - for p := int64(0); p <= 100; p++ { - select { - case <-stop: - return - default: - } - fromPubkeyBackfillSetProcessed(p) - } - fromPubkeyBackfillMarkDone() - fromPubkeyBackfillReset() - } - }() - - // Readers: hammer healthz, assert invariants on each response. - const readers = 8 - const reads = 200 - errs := make(chan string, readers*reads) - for i := 0; i < readers; i++ { - readerWg.Add(1) - go func() { - defer readerWg.Done() - for j := 0; j < reads; j++ { - req := httptest.NewRequest("GET", "/api/healthz", nil) - w := httptest.NewRecorder() - srv.handleHealthz(w, req) - var resp map[string]interface{} - if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { - errs <- "invalid JSON: " + err.Error() - return - } - bf, _ := resp["from_pubkey_backfill"].(map[string]interface{}) - total, _ := bf["total"].(float64) - processed, _ := bf["processed"].(float64) - done, _ := bf["done"].(bool) - if processed > total { - errs <- "processed>total snapshot: processed=" + ftoa(processed) + " total=" + ftoa(total) - return - } - if done && processed != total { - errs <- "done=true but processed!=total: processed=" + ftoa(processed) + " total=" + ftoa(total) - return - } - } - }() - } - - // Wait for readers to complete (bounded by 'reads' iterations), then - // stop the writer and drain. - readerDone := make(chan struct{}) - go func() { readerWg.Wait(); close(readerDone) }() - select { - case <-readerDone: - case <-time.After(5 * time.Second): - close(stop) - writerWg.Wait() - t.Fatal("timed out waiting for reader goroutines") - } - close(stop) - writerWg.Wait() - - close(errs) - for e := range errs { - t.Errorf("inconsistent snapshot: %s", e) - } -} - -func ftoa(f float64) string { return fmt.Sprintf("%g", f) } diff --git a/cmd/server/main.go b/cmd/server/main.go index 359b3e96..7bb27b06 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -18,6 +18,7 @@ import ( "time" "github.com/gorilla/mux" + "github.com/meshcore-analyzer/dbschema" ) // Set via -ldflags at build time @@ -170,10 +171,12 @@ func main() { // auto_vacuum is checked + migrated by the ingestor (#1283). The // server is read-only and must not race the writer for the lock. - // Ensure indexes the server's SQL fallback path depends on - // (mirrors ingestor schema for DBs created by old server-only builds). - if err := ensureServerIndexes(resolvedDB); err != nil { - log.Printf("[db] warning: could not ensure server indexes: %v", err) + // Assert all schema migrations the ingestor owns have already run + // (#1287). The server NEVER migrates — it only reads. If a required + // column/index/table is missing, the operator must restart the + // ingestor (which owns dbschema.Apply) before this server can start. + if err := dbschema.AssertReady(database.conn); err != nil { + log.Fatalf("[db] schema not ready (ingestor must run migrations first): %v", err) } // In-memory packet store @@ -187,63 +190,12 @@ func main() { go store.loadBackgroundChunks() } - // Initialize persisted neighbor graph + // Initialize persisted neighbor graph. + // Per #1287, schema migrations all live in the ingestor (see + // dbschema.Apply). The server merely loads the snapshot here and + // then refreshes it via the recompNeighborGraph slot every 60s. dbPath = database.path - if err := ensureNeighborEdgesTable(dbPath); err != nil { - log.Printf("[neighbor] warning: could not create neighbor_edges table: %v", err) - } - // Add resolved_path column if missing. - // NOTE on startup ordering (review item #10): ensureResolvedPathColumn runs AFTER - // OpenDB/detectSchema, so db.hasResolvedPath will be false on first run with a - // pre-existing DB. This means Load() won't SELECT resolved_path from SQLite. - // Async backfill runs after HTTP starts (see backfillResolvedPathsAsync below) - // AND to SQLite. On next restart, detectSchema finds the column and Load() reads it. - if err := ensureResolvedPathColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add resolved_path column: %v", err) - } else { - database.hasResolvedPath = true // detectSchema ran before column was added; fix the flag - } - - // Ensure observers.inactive column exists (PR #954 filters on it; ingestor migration - // adds it but server may run against DBs ingestor never touched, e.g. e2e fixture). - if err := ensureObserverInactiveColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add observers.inactive column: %v", err) - } - - // Ensure observers.last_packet_at column exists (PR #905 reads it; ingestor migration - // adds it but server may run against DBs ingestor never touched, e.g. e2e fixture). - if err := ensureLastPacketAtColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add observers.last_packet_at column: %v", err) - } - - // Ensure observers.iata column exists (#1188 read paths COALESCE(obs.iata, '') - // in Store.Load() / IngestNewFromDB / IngestNewObservations; ingestor migration - // adds it but server may run against DBs ingestor never touched (e2e fixture) - // OR pre-iata operator DBs upgraded to this build — without this migration - // the first SELECT crashes with "no such column: obs.iata" (#1189 R1). - if err := ensureObserverIATAColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add observers.iata column: %v", err) - } - - // Ensure nodes.foreign_advert column exists (#730 reads it on every /api/nodes - // scan; ingestor migration foreign_advert_v1 adds it but server may run against - // DBs ingestor never touched, e.g. e2e fixture). - if err := ensureForeignAdvertColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add nodes.foreign_advert column: %v", err) - } - - // Ensure transmissions.from_pubkey column + index exists (#1143). Backfill - // for legacy NULL rows runs async after HTTP starts so it can't block boot - // even on prod-sized DBs (100K+ transmissions). - if err := ensureFromPubkeyColumn(dbPath); err != nil { - log.Printf("[store] warning: could not add transmissions.from_pubkey column: %v", err) - } - - // Soft-delete observers that are in the blacklist (mark inactive=1) so - // historical data from a prior unblocked window is hidden too. - if len(cfg.ObserverBlacklist) > 0 { - softDeleteBlacklistedObservers(dbPath, cfg.ObserverBlacklist) - } + database.hasResolvedPath = true // dbschema.AssertReady above already verified observations.resolved_path exists // WaitGroup for background init steps that gate /api/healthz readiness. var initWg sync.WaitGroup @@ -253,8 +205,14 @@ func main() { store.graph.Store(loadNeighborEdgesFromDB(database.conn)) log.Printf("[neighbor] loaded persisted neighbor graph") } else { - log.Printf("[neighbor] no persisted edges found, will build in background...") - store.graph.Store(NewNeighborGraph()) // empty graph — gets populated by background goroutine + // No persisted snapshot yet (e.g. fresh DB before the ingestor + // has run its first edge-build cycle). Build an in-memory graph + // from the packets we already have so reads aren't empty. We + // do NOT persist — the ingestor owns neighbor_edges writes per + // #1287; the recompNeighborGraph recomputer will pick up the + // real snapshot as soon as the ingestor populates it. + log.Printf("[neighbor] no persisted edges found, will build in-memory in background...") + store.graph.Store(NewNeighborGraph()) initWg.Add(1) go func() { defer initWg.Done() @@ -263,14 +221,9 @@ func main() { log.Printf("[neighbor] graph build panic recovered: %v", r) } }() - rw, rwErr := cachedRW(dbPath) - if rwErr == nil { - edgeCount := buildAndPersistEdges(store, rw) - log.Printf("[neighbor] persisted %d edges", edgeCount) - } built := BuildFromStore(store) store.graph.Store(built) - log.Printf("[neighbor] graph build complete") + log.Printf("[neighbor] in-memory graph build complete") }() } @@ -387,40 +340,22 @@ func main() { log.Printf("[bridge-recompute] background recompute enabled (interval=%s)", cfg.AnalyticsDefaultRecomputeInterval()) + // Steady-state neighbor-graph snapshot recomputer (issue #1287). + // Per Option 4: the ingestor owns neighbor_edges; the server + // READS the snapshot every 60s and atomic-swaps it into s.graph. + // This is the ONLY path that updates s.graph at steady state. + stopNeighborRecomp := store.StartNeighborGraphRecomputer(NeighborGraphRecomputerDefaultInterval) + defer stopNeighborRecomp() + log.Printf("[neighbor-recompute] snapshot reload enabled (interval=%s)", + NeighborGraphRecomputerDefaultInterval) + // Packet / metrics / observer retention moved to the ingestor in - // #1283 (writes only belong on the writer process). The server no - // longer schedules any of these; the ingestor's tickers handle them. + // #1283 (writes only belong on the writer process). Neighbor-edge + // pruning moved to the ingestor in #1287 for the same reason. The + // server no longer schedules any of these; the ingestor's tickers + // handle them. _ = cfg.IncrementalVacuumPages() // kept reachable for config validation; not used here - var stopEdgePrune func() - { - maxAgeDays := cfg.NeighborMaxAgeDays() - edgePruneTicker := time.NewTicker(24 * time.Hour) - edgePruneDone := make(chan struct{}) - stopEdgePrune = func() { - edgePruneTicker.Stop() - close(edgePruneDone) - } - go func() { - defer func() { - if r := recover(); r != nil { - log.Printf("[neighbor-prune] panic recovered: %v", r) - } - }() - time.Sleep(4 * time.Minute) // stagger after metrics prune - g := store.graph.Load() - PruneNeighborEdges(dbPath, g, maxAgeDays) - for { - select { - case <-edgePruneTicker.C: - g := store.graph.Load() - PruneNeighborEdges(dbPath, g, maxAgeDays) - case <-edgePruneDone: - return - } - } - }() - log.Printf("[neighbor-prune] auto-prune enabled: edges older than %d days", maxAgeDays) - } + _ = cfg.NeighborMaxAgeDays() // ditto — owned by ingestor now // Graceful shutdown httpServer := &http.Server{ @@ -440,11 +375,8 @@ func main() { // 1. Stop accepting new WebSocket/poll data poller.Stop() - // 1b. Stop auto-prune ticker (server-side packet/metrics/observer - // prunes were removed in #1283; only neighbor-edge prune remains.) - if stopEdgePrune != nil { - stopEdgePrune() - } + // 1b. Auto-prune tickers were all relocated to the ingestor in + // #1283/#1287 — nothing to stop here. // 1c. Stop steady-state analytics recomputers (issue #1240). // Must happen before dbClose so any in-flight compute that @@ -472,13 +404,10 @@ func main() { log.Printf("[server] CoreScope (Go) listening on http://localhost:%d", cfg.Port) - // Start async backfill in background — HTTP is now available. - go backfillResolvedPathsAsync(store, dbPath, 5000, 100*time.Millisecond, cfg.BackfillHours()) - // #1143: backfill from_pubkey for legacy ADVERT rows. Async so even - // 100K+ rows can't block boot; queries handle NULL gracefully. - // startFromPubkeyBackfill wraps the goroutine dispatch so the async - // contract is testable (see TestBackfillFromPubkey_DoesNotBlockBoot). - startFromPubkeyBackfill(dbPath, 5000, 100*time.Millisecond) + // Backfills (resolved_path, from_pubkey) moved to the ingestor in + // #1287 — they are write operations and belong on the writer + // process. The server reads the results via the periodic + // recompNeighborGraph / fetchResolvedPathForObs paths. // Migrate old content hashes in background (one-time, idempotent). go migrateContentHashesAsync(store, 5000, 100*time.Millisecond) diff --git a/cmd/server/neighbor_persist.go b/cmd/server/neighbor_persist.go index c1cbf373..54a40cc8 100644 --- a/cmd/server/neighbor_persist.go +++ b/cmd/server/neighbor_persist.go @@ -1,42 +1,33 @@ +// Package main: read-only neighbor-edges loader. +// +// Per issue #1287 (followup to #1283), cmd/server is the read path: it +// LOADS the in-memory neighbor graph from the SQLite snapshot the +// ingestor maintains, but never writes to it. The previous write-side +// helpers in this file (buildAndPersistEdges, asyncPersistResolvedPaths +// AndEdges, ensure*Column, softDeleteBlacklistedObservers, +// PruneNeighborEdges, openRW) all moved to cmd/ingestor; cmd/ingestor +// owns CREATE/ALTER/INSERT/UPDATE/DELETE on neighbor_edges and the +// observations/resolved_path column. +// +// Server now refreshes its in-memory copy of the graph via the +// recompNeighborGraph slot in analytics_recomputer.go: every 60s it +// re-reads neighbor_edges and atomic-swaps the resulting NeighborGraph +// into s.graph. package main import ( "database/sql" "encoding/json" - "fmt" "log" "strings" "time" ) -// persistSem limits concurrent async persistence goroutines to 1. -// Without this, each ingest cycle spawns a goroutine that opens a new -// SQLite RW connection; under sustained load goroutines pile up with -// no backpressure, causing contention and busy-timeout cascades. -var persistSem = make(chan struct{}, 1) - -// ─── neighbor_edges table ────────────────────────────────────────────────────── - -// ensureNeighborEdgesTable creates the neighbor_edges table if it doesn't exist. -// Uses a separate read-write connection since the main DB is read-only. -func ensureNeighborEdgesTable(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return fmt.Errorf("open rw for neighbor_edges: %w", err) - } - - _, err = rw.Exec(`CREATE TABLE IF NOT EXISTS neighbor_edges ( - node_a TEXT NOT NULL, - node_b TEXT NOT NULL, - count INTEGER DEFAULT 1, - last_seen TEXT, - PRIMARY KEY (node_a, node_b) - )`) - return err -} +// ─── neighbor_edges loader (read-only) ───────────────────────────────────────── // loadNeighborEdgesFromDB loads all edges from the neighbor_edges table -// and builds an in-memory NeighborGraph. +// and builds an in-memory NeighborGraph. Called on server startup and +// from the recompNeighborGraph background recomputer (#1287). func loadNeighborEdgesFromDB(conn *sql.DB) *NeighborGraph { g := NewNeighborGraph() @@ -59,7 +50,6 @@ func loadNeighborEdgesFromDB(conn *sql.DB) *NeighborGraph { if lastSeen.Valid { ts = parseTimestamp(lastSeen.String) } - // Build edge directly (both nodes are full pubkeys from persisted data) key := makeEdgeKey(a, b) g.mu.Lock() e, exists := g.edges[key] @@ -95,391 +85,28 @@ func loadNeighborEdgesFromDB(conn *sql.DB) *NeighborGraph { return g } -// ─── shared async persistence helper ─────────────────────────────────────────── - -// persistObsUpdate holds data for a resolved_path SQLite update. -type persistObsUpdate struct { - obsID int - resolvedPath string -} - -// persistEdgeUpdate holds data for a neighbor_edges SQLite upsert. -type persistEdgeUpdate struct { - a, b, ts string -} - -// asyncPersistResolvedPathsAndEdges writes resolved_path updates and neighbor -// edge upserts to SQLite in a background goroutine. Shared between -// IngestNewFromDB and IngestNewObservations to avoid DRY violation. -func asyncPersistResolvedPathsAndEdges(dbPath string, obsUpdates []persistObsUpdate, edgeUpdates []persistEdgeUpdate, logPrefix string) { - if len(obsUpdates) == 0 && len(edgeUpdates) == 0 { - return - } - // Try-acquire semaphore BEFORE spawning goroutine. If another - // persistence operation is already running, drop this batch — - // data lives in memory and will be backfilled on restart. - select { - case persistSem <- struct{}{}: - // Acquired — spawn goroutine to do the work. - default: - log.Printf("[store] %s skipped: persistence already in progress", logPrefix) - return - } - go func() { - defer func() { <-persistSem }() - - rw, err := cachedRW(dbPath) - if err != nil { - log.Printf("[store] %s rw open error: %v", logPrefix, err) - return - } - - if len(obsUpdates) > 0 { - sqlTx, err := rw.Begin() - if err == nil { - stmt, err := sqlTx.Prepare("UPDATE observations SET resolved_path = ? WHERE id = ?") - if err == nil { - var firstErr error - for _, u := range obsUpdates { - if _, err := stmt.Exec(u.resolvedPath, u.obsID); err != nil && firstErr == nil { - firstErr = err - } - } - stmt.Close() - if firstErr != nil { - log.Printf("[store] %s resolved_path error (first): %v", logPrefix, firstErr) - } - } else { - log.Printf("[store] %s resolved_path prepare error: %v", logPrefix, err) - } - sqlTx.Commit() - } - } - - if len(edgeUpdates) > 0 { - sqlTx, err := rw.Begin() - if err == nil { - stmt, err := sqlTx.Prepare(`INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) - VALUES (?, ?, 1, ?) - ON CONFLICT(node_a, node_b) DO UPDATE SET - count = count + 1, last_seen = MAX(last_seen, excluded.last_seen)`) - if err == nil { - var firstErr error - for _, e := range edgeUpdates { - if _, err := stmt.Exec(e.a, e.b, e.ts); err != nil && firstErr == nil { - firstErr = err - } - } - stmt.Close() - if firstErr != nil { - log.Printf("[store] %s edge error (first): %v", logPrefix, firstErr) - } - } else { - log.Printf("[store] %s edge prepare error: %v", logPrefix, err) - } - sqlTx.Commit() - } - } - }() -} - -// neighborEdgesTableExists checks if the neighbor_edges table has any data. +// neighborEdgesTableExists returns true when neighbor_edges contains at +// least one row. Used by main.go to decide between "load snapshot" and +// "start with empty graph and wait for the ingestor to populate it". func neighborEdgesTableExists(conn *sql.DB) bool { var cnt int err := conn.QueryRow("SELECT COUNT(*) FROM neighbor_edges").Scan(&cnt) if err != nil { - return false // table doesn't exist + return false } return cnt > 0 } -// buildAndPersistEdges scans all packets in the store, extracts edges per -// ADVERT/non-ADVERT rules, and persists them to SQLite. -func buildAndPersistEdges(store *PacketStore, rw *sql.DB) int { - store.mu.RLock() - packets := make([]*StoreTx, len(store.packets)) - copy(packets, store.packets) - store.mu.RUnlock() +// ─── resolved_path helpers (read-only / in-memory only) ──────────────────────── - _, pm := store.getCachedNodesAndPM() - - tx, err := rw.Begin() - if err != nil { - log.Printf("[neighbor] begin tx error: %v", err) - return 0 - } - defer tx.Rollback() - - stmt, err := tx.Prepare(`INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) - VALUES (?, ?, 1, ?) - ON CONFLICT(node_a, node_b) DO UPDATE SET - count = count + 1, last_seen = MAX(last_seen, excluded.last_seen)`) - if err != nil { - log.Printf("[neighbor] prepare stmt error: %v", err) - return 0 - } - defer stmt.Close() - - edgeCount := 0 - var firstErr error - for _, pkt := range packets { - for _, obs := range pkt.Observations { - for _, ec := range extractEdgesFromObs(obs, pkt, pm) { - if _, err := stmt.Exec(ec.A, ec.B, ec.Timestamp); err != nil && firstErr == nil { - firstErr = err - } - edgeCount++ - } - } - } - if firstErr != nil { - log.Printf("[neighbor] edge exec error (first): %v", firstErr) - } - - if err := tx.Commit(); err != nil { - log.Printf("[neighbor] commit error: %v", err) - return 0 - } - return edgeCount -} - -// ─── resolved_path column ────────────────────────────────────────────────────── - -// ensureResolvedPathColumn adds the resolved_path column to observations if missing. -func ensureResolvedPathColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - - // Check if column already exists - rows, err := rw.Query("PRAGMA table_info(observations)") - if err != nil { - return err - } - defer rows.Close() - - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "resolved_path" { - return nil // already exists - } - } - - _, err = rw.Exec("ALTER TABLE observations ADD COLUMN resolved_path TEXT") - if err != nil { - return fmt.Errorf("add resolved_path column: %w", err) - } - log.Println("[store] Added resolved_path column to observations") - return nil -} - -// ensureObserverInactiveColumn adds the inactive column to observers if missing. -// The column was originally added by ingestor migration (cmd/ingestor/db.go:344) to -// support soft-delete via RemoveStaleObservers + filtered reads (PR #954). When the -// server starts against a DB that was never touched by the ingestor (e.g. the e2e -// fixture), the column is missing and read queries that filter on it (GetObservers, -// GetStats) silently fail with "no such column: inactive" — leaving /api/observers -// returning empty. -func ensureObserverInactiveColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - - rows, err := rw.Query("PRAGMA table_info(observers)") - if err != nil { - return err - } - defer rows.Close() - - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "inactive" { - return nil // already exists - } - } - - _, err = rw.Exec("ALTER TABLE observers ADD COLUMN inactive INTEGER DEFAULT 0") - if err != nil { - return fmt.Errorf("add inactive column: %w", err) - } - log.Println("[store] Added inactive column to observers") - return nil -} - -// ensureLastPacketAtColumn adds the last_packet_at column to observers if missing. -// The column was originally added by ingestor migration (observers_last_packet_at_v1) -// to track the most recent packet observation time separately from status updates. -// When the server starts against a DB that was never touched by the ingestor (e.g. -// the e2e fixture), the column is missing and read queries that reference it -// (GetObservers, GetObserverByID) fail with "no such column: last_packet_at". -func ensureLastPacketAtColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - - rows, err := rw.Query("PRAGMA table_info(observers)") - if err != nil { - return err - } - defer rows.Close() - - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "last_packet_at" { - return nil // already exists - } - } - - _, err = rw.Exec("ALTER TABLE observers ADD COLUMN last_packet_at TEXT") - if err != nil { - return fmt.Errorf("add last_packet_at column: %w", err) - } - log.Println("[store] Added last_packet_at column to observers") - return nil -} - -// ensureObserverIATAColumn adds the iata column to observers if missing. -// The column was originally added by ingestor migration (cmd/ingestor/db.go) to -// label each observer with a 3-letter regional IATA code. When the server starts -// against a DB that was never touched by the ingestor (e.g. the e2e fixture, -// or a pre-iata operator DB upgraded to this build), every SELECT that joins -// COALESCE(obs.iata, '') panics with "no such column: obs.iata" — crashing -// Store.Load() / IngestNewFromDB / IngestNewObservations on startup (#1189 R1). -func ensureObserverIATAColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - - rows, err := rw.Query("PRAGMA table_info(observers)") - if err != nil { - return err - } - defer rows.Close() - - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "iata" { - return nil // already exists - } - } - - _, err = rw.Exec("ALTER TABLE observers ADD COLUMN iata TEXT") - if err != nil { - return fmt.Errorf("add iata column: %w", err) - } - log.Println("[store] Added iata column to observers") - return nil -} - -// ensureForeignAdvertColumn adds the foreign_advert column to nodes/inactive_nodes -// if missing (#730). The column is added by the ingestor migration foreign_advert_v1 -// — but the server may run against a DB the ingestor has never touched (e2e fixture, -// fresh installs where the server boots first), in which case scanNodeRow fails -// with "no such column: foreign_advert" and /api/nodes silently returns nothing. -func ensureForeignAdvertColumn(dbPath string) error { - rw, err := cachedRW(dbPath) - if err != nil { - return err - } - for _, table := range []string{"nodes", "inactive_nodes"} { - has, err := tableHasColumn(rw, table, "foreign_advert") - if err != nil { - return fmt.Errorf("inspect %s: %w", table, err) - } - if has { - continue - } - if _, err := rw.Exec(fmt.Sprintf("ALTER TABLE %s ADD COLUMN foreign_advert INTEGER DEFAULT 0", table)); err != nil { - return fmt.Errorf("add foreign_advert to %s: %w", table, err) - } - log.Printf("[store] Added foreign_advert column to %s", table) - } - return nil -} - -// tableHasColumn reports whether the named table has the named column. -func tableHasColumn(rw *sql.DB, table, column string) (bool, error) { - rows, err := rw.Query(fmt.Sprintf("PRAGMA table_info(%s)", table)) - if err != nil { - return false, err - } - defer rows.Close() - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == column { - return true, nil - } - } - return false, nil -} - -// softDeleteBlacklistedObservers marks observers matching the blacklist as -// inactive=1 so they are hidden from API responses. Runs once at startup. -func softDeleteBlacklistedObservers(dbPath string, blacklist []string) { - rw, err := cachedRW(dbPath) - if err != nil { - log.Printf("[observer-blacklist] warning: could not open DB for soft-delete: %v", err) - return - } - - placeholders := make([]string, 0, len(blacklist)) - args := make([]interface{}, 0, len(blacklist)) - for _, pk := range blacklist { - trimmed := strings.TrimSpace(pk) - if trimmed == "" { - continue - } - placeholders = append(placeholders, "LOWER(?)") - args = append(args, trimmed) - } - if len(placeholders) == 0 { - return - } - - query := "UPDATE observers SET inactive = 1 WHERE LOWER(id) IN (" + strings.Join(placeholders, ",") + ") AND (inactive IS NULL OR inactive = 0)" - result, err := rw.Exec(query, args...) - if err != nil { - log.Printf("[observer-blacklist] warning: soft-delete failed: %v", err) - return - } - if n, _ := result.RowsAffected(); n > 0 { - log.Printf("[observer-blacklist] soft-deleted %d blacklisted observer(s)", n) - } -} - -// resolvePathForObs resolves hop prefixes to full pubkeys for an observation. -// Returns nil if path is empty. +// resolvePathForObs resolves hop prefixes to full pubkeys for an +// observation. Pure compute — does NOT persist (the ingestor owns +// writes to observations.resolved_path). func resolvePathForObs(pathJSON, observerID string, tx *StoreTx, pm *prefixMap, graph *NeighborGraph) []*string { hops := parsePathJSON(pathJSON) if len(hops) == 0 { return nil } - - // Build context pubkeys: observer + originator (if known) contextPKs := make([]string, 0, 3) if observerID != "" { contextPKs = append(contextPKs, strings.ToLower(observerID)) @@ -488,28 +115,23 @@ func resolvePathForObs(pathJSON, observerID string, tx *StoreTx, pm *prefixMap, if fromNode != "" { contextPKs = append(contextPKs, strings.ToLower(fromNode)) } - resolved := make([]*string, len(hops)) for i, hop := range hops { - // Add adjacent hops as context for disambiguation ctx := make([]string, len(contextPKs), len(contextPKs)+2) copy(ctx, contextPKs) - // Add previously resolved hops as context if i > 0 && resolved[i-1] != nil { ctx = append(ctx, *resolved[i-1]) } - node, _, _ := pm.resolveWithContext(hop, ctx, graph) if node != nil { pk := strings.ToLower(node.PublicKey) resolved[i] = &pk } } - return resolved } -// marshalResolvedPath converts []*string to JSON for storage. +// marshalResolvedPath converts []*string to JSON for in-memory caching. func marshalResolvedPath(rp []*string) string { if len(rp) == 0 { return "" @@ -533,225 +155,22 @@ func unmarshalResolvedPath(s string) []*string { return result } +// ─── Shared edge-extraction helper (used by ingestor + tests) ────────────────── -// backfillResolvedPathsAsync processes observations with NULL resolved_path in -// chunks, yielding between batches so HTTP handlers remain responsive. It sets -// store.backfillComplete when finished and re-picks best observations for any -// transmissions affected by newly resolved paths. -func backfillResolvedPathsAsync(store *PacketStore, dbPath string, chunkSize int, yieldDuration time.Duration, backfillHours int) { - defer func() { - if r := recover(); r != nil { - log.Printf("[store] backfillResolvedPathsAsync panic recovered: %v", r) - } - }() - // Collect ALL pending obs refs upfront in one pass under a single RLock (fix A). - type obsRef struct { - obsID int - pathJSON string - observerID string - txJSON string - payloadType *int - txHash string // to re-pick best obs - } - - cutoff := time.Now().UTC().Add(-time.Duration(backfillHours) * time.Hour) - - store.mu.RLock() - pm := store.nodePM - var allPending []obsRef - for _, tx := range store.packets { - // Skip transmissions older than the backfill window. - if tx.FirstSeen != "" { - if ts, err := time.Parse(time.RFC3339Nano, tx.FirstSeen); err == nil && ts.Before(cutoff) { - continue - } - // Also try the common SQLite format - if ts, err := time.Parse("2006-01-02 15:04:05", tx.FirstSeen); err == nil && ts.Before(cutoff) { - continue - } - } - for _, obs := range tx.Observations { - // Check if this observation has been resolved: look up in the index. - // If the tx has no reverse-map entries AND path is non-empty, it needs backfill. - hasRP := false - if _, ok := store.resolvedPubkeyReverse[tx.ID]; ok { - hasRP = true - } - if !hasRP && obs.PathJSON != "" && obs.PathJSON != "[]" { - allPending = append(allPending, obsRef{ - obsID: obs.ID, - pathJSON: obs.PathJSON, - observerID: obs.ObserverID, - txJSON: tx.DecodedJSON, - payloadType: tx.PayloadType, - txHash: tx.Hash, - }) - } - } - } - store.mu.RUnlock() - - totalPending := len(allPending) - if totalPending == 0 || pm == nil { - store.backfillComplete.Store(true) - log.Printf("[store] async resolved_path backfill: nothing to do") - return - } - - store.backfillTotal.Store(int64(totalPending)) - store.backfillProcessed.Store(0) - log.Printf("[store] async resolved_path backfill starting: %d observations", totalPending) - - // Open RW connection once before the chunk loop (fix B). - var rw *sql.DB - if dbPath != "" { - var err error - rw, err = cachedRW(dbPath) - if err != nil { - log.Printf("[store] async backfill: open rw error: %v", err) - } - } - // rw is cached process-wide; do not close - - totalProcessed := 0 - for totalProcessed < totalPending { - end := totalProcessed + chunkSize - if end > totalPending { - end = totalPending - } - chunk := allPending[totalProcessed:end] - - // Re-read graph at the start of each chunk so we pick up a freshly- - // built graph once the background build goroutine completes, instead - // of using the potentially-empty graph captured at cold start. - // (Dead RLock wrap removed PR #1208: store.graph is atomic.Pointer.) - graph := store.graph.Load() - - // Resolve paths outside any lock. - type resolved struct { - obsID int - rp []*string - rpJSON string - txHash string - } - var results []resolved - for _, ref := range chunk { - fakeTx := &StoreTx{DecodedJSON: ref.txJSON, PayloadType: ref.payloadType} - rp := resolvePathForObs(ref.pathJSON, ref.observerID, fakeTx, pm, graph) - if len(rp) > 0 { - rpJSON := marshalResolvedPath(rp) - if rpJSON != "" { - results = append(results, resolved{ref.obsID, rp, rpJSON, ref.txHash}) - } - } - } - - // Persist to SQLite using the shared connection. - if len(results) > 0 && rw != nil { - sqlTx, err := rw.Begin() - if err != nil { - log.Printf("[store] async backfill: begin tx error: %v", err) - } else { - stmt, err := sqlTx.Prepare("UPDATE observations SET resolved_path = ? WHERE id = ?") - if err != nil { - log.Printf("[store] async backfill: prepare error: %v", err) - sqlTx.Rollback() - } else { - var execErr error - for _, r := range results { - if _, e := stmt.Exec(r.rpJSON, r.obsID); e != nil && execErr == nil { - execErr = e - } - } - if execErr != nil { - log.Printf("[store] async backfill: exec error (first): %v", execErr) - } - stmt.Close() - if err := sqlTx.Commit(); err != nil { - log.Printf("[store] async backfill: commit error: %v", err) - } - } - } - - // Update in-memory state: update resolved pubkey index, re-pick best observation, - // and invalidate LRU cache entries for backfilled observations (#800). - // - // Lock ordering: always take s.mu BEFORE lruMu. The read path - // (fetchResolvedPathForObs) takes lruMu independently of s.mu, - // so we must NOT hold s.mu while taking lruMu. Instead, collect - // obsIDs to invalidate under s.mu, release it, then take lruMu. - store.mu.Lock() - affectedSet := make(map[string]bool) - lruInvalidate := make([]int, 0, len(results)) - for _, r := range results { - // Remove old index entries for this tx, then re-add with new pubkeys - if !affectedSet[r.txHash] { - affectedSet[r.txHash] = true - if tx, ok := store.byHash[r.txHash]; ok { - store.removeFromResolvedPubkeyIndex(tx.ID) - } - } - // Add new resolved pubkeys to index - if tx, ok := store.byHash[r.txHash]; ok { - pks := extractResolvedPubkeys(r.rp) - store.addToResolvedPubkeyIndex(tx.ID, pks) - // Update byNode for relay nodes - for _, pk := range pks { - store.addToByNode(tx, pk) - } - // Update byPathHop resolved-key entries - hopsSeen := make(map[string]bool) - for _, hop := range txGetParsedPath(tx) { - hopsSeen[strings.ToLower(hop)] = true - } - for _, pk := range pks { - if !hopsSeen[pk] { - hopsSeen[pk] = true - store.byPathHop[pk] = append(store.byPathHop[pk], tx) - } - } - } - lruInvalidate = append(lruInvalidate, r.obsID) - } - // Re-pick best observation for affected transmissions - for txHash := range affectedSet { - if tx, ok := store.byHash[txHash]; ok { - pickBestObservation(tx) - } - } - store.mu.Unlock() - - // Invalidate LRU entries AFTER releasing s.mu to maintain lock - // ordering (lruMu must never be taken while s.mu is held). - store.lruMu.Lock() - for _, obsID := range lruInvalidate { - store.lruDelete(obsID) - } - store.lruMu.Unlock() - } - - totalProcessed += len(chunk) - store.backfillProcessed.Store(int64(totalProcessed)) - pct := float64(totalProcessed) / float64(totalPending) * 100 - log.Printf("[store] backfill progress: %d/%d observations (%.1f%%)", totalProcessed, totalPending, pct) - - time.Sleep(yieldDuration) - } - - store.backfillComplete.Store(true) - log.Printf("[store] async resolved_path backfill complete: %d observations processed", totalProcessed) -} - -// ─── Shared helpers ──────────────────────────────────────────────────────────── - -// edgeCandidate represents an extracted edge to be persisted. +// edgeCandidate represents an extracted edge. The ingestor uses the +// same logic when computing edges from observations. type edgeCandidate struct { A, B, Timestamp string } -// extractEdgesFromObs extracts neighbor edge candidates from a single observation. -// For ADVERTs: originator↔path[0] (if unambiguous). For ALL types: observer↔path[last] (if unambiguous). -// Also handles zero-hop ADVERTs (originator↔observer direct link). +// extractEdgesFromObs extracts neighbor edge candidates from a single +// observation. For ADVERTs: originator↔path[0] (if unambiguous). For +// ALL types: observer↔path[last] (if unambiguous). Also handles +// zero-hop ADVERTs (originator↔observer direct link). +// +// Kept in cmd/server because the in-memory graph builder +// (neighbor_graph.go) also calls it; it is pure compute and does not +// touch the DB. func extractEdgesFromObs(obs *StoreObs, tx *StoreTx, pm *prefixMap) []edgeCandidate { isAdvert := tx.PayloadType != nil && *tx.PayloadType == PayloadADVERT fromNode := extractFromNode(tx) @@ -774,7 +193,6 @@ func extractEdgesFromObs(obs *StoreObs, tx *StoreTx, pm *prefixMap) []edgeCandid return edges } - // Edge 1: originator ↔ path[0] — ADVERTs only (resolve prefix to full pubkey) if isAdvert && fromNode != "" && pm != nil { firstHop := strings.ToLower(path[0]) fromLower := strings.ToLower(fromNode) @@ -791,7 +209,6 @@ func extractEdgesFromObs(obs *StoreObs, tx *StoreTx, pm *prefixMap) []edgeCandid } } - // Edge 2: observer ↔ path[last] — ALL packet types if pm != nil { lastHop := strings.ToLower(path[len(path)-1]) candidates := pm.m[lastHop] @@ -809,50 +226,3 @@ func extractEdgesFromObs(obs *StoreObs, tx *StoreTx, pm *prefixMap) []edgeCandid return edges } - -// openRW opens a read-write SQLite connection (same pattern as PruneOldPackets). -func openRW(dbPath string) (*sql.DB, error) { - dsn := fmt.Sprintf("file:%s?_journal_mode=WAL", dbPath) - rw, err := sql.Open("sqlite", dsn) - if err != nil { - return nil, err - } - rw.SetMaxOpenConns(1) - // DSN _busy_timeout may not be honored by all drivers; set via PRAGMA - // to guarantee SQLite retries for up to 5s before returning SQLITE_BUSY. - if _, err := rw.Exec("PRAGMA busy_timeout = 5000"); err != nil { - rw.Close() - return nil, fmt.Errorf("set busy_timeout: %w", err) - } - return rw, nil -} - -// PruneNeighborEdges removes edges older than maxAgeDays from both SQLite and -// the in-memory graph. Uses openRW internally because the shared database.conn -// is opened with mode=ro and DELETEs would silently fail. -func PruneNeighborEdges(dbPath string, graph *NeighborGraph, maxAgeDays int) (int, error) { - cutoff := time.Now().UTC().Add(-time.Duration(maxAgeDays) * 24 * time.Hour) - - // 1. Prune from SQLite using a read-write connection - var dbPruned int64 - rw, err := cachedRW(dbPath) - if err != nil { - return 0, fmt.Errorf("prune neighbor_edges: open rw: %w", err) - } - res, err := rw.Exec("DELETE FROM neighbor_edges WHERE last_seen < ?", cutoff.Format(time.RFC3339)) - if err != nil { - return 0, fmt.Errorf("prune neighbor_edges: %w", err) - } - dbPruned, _ = res.RowsAffected() - - // 2. Prune from in-memory graph - memPruned := 0 - if graph != nil { - memPruned = graph.PruneOlderThan(cutoff) - } - - if dbPruned > 0 || memPruned > 0 { - log.Printf("[neighbor-prune] removed %d DB rows, %d in-memory edges older than %d days", dbPruned, memPruned, maxAgeDays) - } - return int(dbPruned), nil -} diff --git a/cmd/server/neighbor_persist_test.go b/cmd/server/neighbor_persist_test.go deleted file mode 100644 index 0e843e56..00000000 --- a/cmd/server/neighbor_persist_test.go +++ /dev/null @@ -1,687 +0,0 @@ -package main - -import ( - "database/sql" - "encoding/json" - "path/filepath" - "strings" - "testing" - "time" - - _ "modernc.org/sqlite" -) - -// createTestDBWithSchema creates a temp SQLite DB with the standard schema + resolved_path column. -func createTestDBWithSchema(t *testing.T) (*DB, string) { - t.Helper() - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - conn, err := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - if err != nil { - t.Fatal(err) - } - - // Create tables - conn.Exec(`CREATE TABLE transmissions ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - raw_hex TEXT, hash TEXT UNIQUE, first_seen TEXT, - route_type INTEGER, payload_type INTEGER, payload_version INTEGER, - decoded_json TEXT, channel_hash TEXT DEFAULT NULL - )`) - conn.Exec(`CREATE TABLE observers ( - id TEXT PRIMARY KEY, name TEXT, iata TEXT - )`) - conn.Exec(`CREATE TABLE observations ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - transmission_id INTEGER NOT NULL REFERENCES transmissions(id), - observer_id TEXT, observer_name TEXT, direction TEXT, - snr REAL, rssi REAL, score INTEGER, - path_json TEXT, timestamp TEXT, - resolved_path TEXT, raw_hex TEXT - )`) - conn.Exec(`CREATE TABLE nodes ( - public_key TEXT PRIMARY KEY, name TEXT, role TEXT, - lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, - advert_count INTEGER DEFAULT 0 - )`) - - conn.Close() - - db, err := OpenDB(dbPath) - if err != nil { - t.Fatal(err) - } - return db, dbPath -} - -func TestResolvePathForObs(t *testing.T) { - // Build a prefix map with known nodes - nodes := []nodeInfo{ - {Role: "repeater", PublicKey: "aabbccddee1234567890aabbccddee1234567890aabbccddee1234567890aabb", Name: "Node-AA"}, - {Role: "repeater", PublicKey: "bbccddee1234567890aabbccddee1234567890aabbccddee1234567890aabb11", Name: "Node-BB"}, - } - pm := buildPrefixMap(nodes) - graph := NewNeighborGraph() - - tx := &StoreTx{ - DecodedJSON: `{"pubKey": "originator1234567890"}`, - PayloadType: intPtr(4), - } - - // Unambiguous prefixes should resolve - rp := resolvePathForObs(`["aa","bb"]`, "observer1", tx, pm, graph) - if len(rp) != 2 { - t.Fatalf("expected 2 resolved hops, got %d", len(rp)) - } - if rp[0] == nil || !strings.HasPrefix(*rp[0], "aabbcc") { - t.Errorf("expected first hop to resolve to Node-AA, got %v", rp[0]) - } - if rp[1] == nil || !strings.HasPrefix(*rp[1], "bbccdd") { - t.Errorf("expected second hop to resolve to Node-BB, got %v", rp[1]) - } -} - -func TestResolvePathForObs_EmptyPath(t *testing.T) { - pm := buildPrefixMap(nil) - rp := resolvePathForObs(`[]`, "", &StoreTx{}, pm, nil) - if rp != nil { - t.Errorf("expected nil for empty path, got %v", rp) - } - - rp = resolvePathForObs("", "", &StoreTx{}, pm, nil) - if rp != nil { - t.Errorf("expected nil for empty string, got %v", rp) - } -} - -func TestResolvePathForObs_Unresolvable(t *testing.T) { - nodes := []nodeInfo{ - {Role: "repeater", PublicKey: "aabbccddee1234567890aabbccddee1234567890aabbccddee1234567890aabb", Name: "Node-AA"}, - } - pm := buildPrefixMap(nodes) - - // "zz" prefix doesn't match any node - rp := resolvePathForObs(`["zz"]`, "", &StoreTx{}, pm, nil) - if len(rp) != 1 { - t.Fatalf("expected 1 hop, got %d", len(rp)) - } - if rp[0] != nil { - t.Errorf("expected nil for unresolvable hop, got %v", *rp[0]) - } -} - -func TestMarshalUnmarshalResolvedPath(t *testing.T) { - pk1 := "aabbccdd" - var rp []*string - rp = append(rp, &pk1, nil) - - j := marshalResolvedPath(rp) - if j == "" { - t.Fatal("expected non-empty JSON") - } - - parsed := unmarshalResolvedPath(j) - if len(parsed) != 2 { - t.Fatalf("expected 2 elements, got %d", len(parsed)) - } - if parsed[0] == nil || *parsed[0] != "aabbccdd" { - t.Errorf("first element wrong: %v", parsed[0]) - } - if parsed[1] != nil { - t.Errorf("second element should be nil, got %v", *parsed[1]) - } -} - -func TestMarshalResolvedPath_Empty(t *testing.T) { - if marshalResolvedPath(nil) != "" { - t.Error("expected empty for nil") - } - if marshalResolvedPath([]*string{}) != "" { - t.Error("expected empty for empty slice") - } -} - -func TestUnmarshalResolvedPath_Invalid(t *testing.T) { - if unmarshalResolvedPath("") != nil { - t.Error("expected nil for empty string") - } - if unmarshalResolvedPath("not json") != nil { - t.Error("expected nil for invalid JSON") - } -} - -func TestEnsureNeighborEdgesTable(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - // Create initial DB - conn, _ := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - conn.Exec("CREATE TABLE test (id INTEGER PRIMARY KEY)") - conn.Close() - - if err := ensureNeighborEdgesTable(dbPath); err != nil { - t.Fatal(err) - } - - // Verify table exists - conn, _ = sql.Open("sqlite", "file:"+dbPath+"?mode=ro") - defer conn.Close() - var cnt int - if err := conn.QueryRow("SELECT COUNT(*) FROM neighbor_edges").Scan(&cnt); err != nil { - t.Fatalf("neighbor_edges table not created: %v", err) - } -} - -func TestLoadNeighborEdgesFromDB(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - conn, _ := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - conn.Exec(`CREATE TABLE neighbor_edges ( - node_a TEXT NOT NULL, node_b TEXT NOT NULL, - count INTEGER DEFAULT 1, last_seen TEXT, - PRIMARY KEY (node_a, node_b) - )`) - conn.Exec("INSERT INTO neighbor_edges VALUES ('aaa', 'bbb', 5, '2024-01-01T00:00:00Z')") - conn.Exec("INSERT INTO neighbor_edges VALUES ('ccc', 'ddd', 3, '2024-01-02T00:00:00Z')") - - g := loadNeighborEdgesFromDB(conn) - conn.Close() - - // Should have 2 edges - edges := g.AllEdges() - if len(edges) != 2 { - t.Errorf("expected 2 edges, got %d", len(edges)) - } - - // Check neighbors - n := g.Neighbors("aaa") - if len(n) != 1 { - t.Errorf("expected 1 neighbor for aaa, got %d", len(n)) - } -} - -func TestStoreObsResolvedPathInBroadcast(t *testing.T) { - // After #800 refactor, resolved_path is no longer stored on StoreTx/StoreObs structs. - // Broadcast maps carry resolved_path from the decode-window, not from struct fields. - // This test verifies pickBestObservation no longer sets ResolvedPath on tx. - obs := &StoreObs{ - ID: 1, - ObserverID: "obs1", - ObserverName: "Observer 1", - PathJSON: `["aa"]`, - Timestamp: "2024-01-01T00:00:00Z", - } - - tx := &StoreTx{ - ID: 1, - Hash: "abc123", - Observations: []*StoreObs{obs}, - } - pickBestObservation(tx) - - // tx should NOT have a ResolvedPath field anymore (compile-time guard) - // Verify the best observation's fields are propagated correctly - if tx.ObserverID != "obs1" { - t.Errorf("expected ObserverID=obs1, got %s", tx.ObserverID) - } -} - -func TestResolvedPathInTxToMap(t *testing.T) { - // After #800, txToMap no longer includes resolved_path from the struct. - // resolved_path is only available via on-demand SQL fetch (txToMapWithRP). - tx := &StoreTx{ - ID: 1, - Hash: "abc123", - PathJSON: `["aa"]`, - obsKeys: make(map[string]bool), - } - - m := txToMap(tx) - if _, ok := m["resolved_path"]; ok { - t.Error("resolved_path should not be in txToMap output (removed in #800)") - } -} - -func TestResolvedPathOmittedWhenNil(t *testing.T) { - tx := &StoreTx{ - ID: 1, - Hash: "abc123", - obsKeys: make(map[string]bool), - } - - m := txToMap(tx) - if _, ok := m["resolved_path"]; ok { - t.Error("resolved_path should not be in map when nil") - } -} - -func TestEnsureResolvedPathColumn(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - conn, _ := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - conn.Exec(`CREATE TABLE observations ( - id INTEGER PRIMARY KEY, transmission_id INTEGER, - observer_id TEXT, path_json TEXT, timestamp TEXT, raw_hex TEXT - )`) - conn.Close() - - if err := ensureResolvedPathColumn(dbPath); err != nil { - t.Fatal(err) - } - - // Verify column exists - conn, _ = sql.Open("sqlite", "file:"+dbPath+"?mode=ro") - defer conn.Close() - rows, _ := conn.Query("PRAGMA table_info(observations)") - found := false - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) - if colName == "resolved_path" { - found = true - } - } - rows.Close() - if !found { - t.Error("resolved_path column not added") - } - - // Running again should be idempotent - if err := ensureResolvedPathColumn(dbPath); err != nil { - t.Fatal("second call should be idempotent:", err) - } -} - -func TestDBDetectsResolvedPathColumn(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - // Create DB without resolved_path - conn, _ := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - conn.Exec(`CREATE TABLE observations (id INTEGER PRIMARY KEY, observer_idx INTEGER)`) - conn.Exec(`CREATE TABLE transmissions (id INTEGER PRIMARY KEY)`) - conn.Close() - - db, err := OpenDB(dbPath) - if err != nil { - t.Fatal(err) - } - if db.hasResolvedPath { - t.Error("should not detect resolved_path when column missing") - } - db.Close() - - // Add resolved_path column - conn, _ = sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - conn.Exec("ALTER TABLE observations ADD COLUMN resolved_path TEXT") - conn.Close() - - db, err = OpenDB(dbPath) - if err != nil { - t.Fatal(err) - } - if !db.hasResolvedPath { - t.Error("should detect resolved_path when column exists") - } - db.Close() -} - -func TestLoadWithResolvedPath(t *testing.T) { - db, dbPath := createTestDBWithSchema(t) - defer db.Close() - - // Insert test data - rw, _ := openRW(dbPath) - rw.Exec(`INSERT INTO transmissions (id, hash, first_seen, payload_type, decoded_json) - VALUES (1, 'hash1', '2024-01-01T00:00:00Z', 4, '{"pubKey":"origpk"}')`) - rw.Exec(`INSERT INTO observations (id, transmission_id, observer_id, observer_name, path_json, timestamp, resolved_path) - VALUES (1, 1, 'obs1', 'Observer1', '["aa"]', '2024-01-01T00:00:00Z', '["aabbccdd"]')`) - rw.Close() - - store := NewPacketStore(db, nil) - if err := store.Load(); err != nil { - t.Fatal(err) - } - - if len(store.packets) != 1 { - t.Fatalf("expected 1 packet, got %d", len(store.packets)) - } - - tx := store.packets[0] - if len(tx.Observations) != 1 { - t.Fatalf("expected 1 observation, got %d", len(tx.Observations)) - } - - // After #800, ResolvedPath is not stored on StoreObs struct. - // Instead, resolved pubkeys are in the membership index. - _ = tx.Observations[0] // obs exists - h := resolvedPubkeyHash("aabbccdd") - if len(store.resolvedPubkeyIndex[h]) != 1 { - t.Fatal("expected resolved pubkey to be indexed") - } -} - -func TestResolvedPathInAPIResponse(t *testing.T) { - // After #800, TransmissionResp no longer has ResolvedPath field. - // resolved_path is included dynamically in map-based API responses. - resp := TransmissionResp{ - ID: 1, - Hash: "test", - } - - data, err := json.Marshal(resp) - if err != nil { - t.Fatal(err) - } - - var m map[string]interface{} - json.Unmarshal(data, &m) - - // resolved_path should NOT be in the marshaled JSON - if _, ok := m["resolved_path"]; ok { - t.Error("resolved_path should not be in TransmissionResp JSON (#800)") - } -} - -func TestResolvedPathOmittedWhenEmpty(t *testing.T) { - resp := TransmissionResp{ - ID: 1, - Hash: "test", - } - - data, _ := json.Marshal(resp) - var m map[string]interface{} - json.Unmarshal(data, &m) - - if _, ok := m["resolved_path"]; ok { - t.Error("resolved_path should be omitted when nil") - } -} - -func TestExtractEdgesFromObs_AdvertNoPath(t *testing.T) { - tx := &StoreTx{ - DecodedJSON: `{"pubKey":"aaaa1111"}`, - PayloadType: intPtr(4), - } - obs := &StoreObs{ - ObserverID: "bbbb2222", - PathJSON: "", - Timestamp: "2024-01-01T00:00:00Z", - } - - edges := extractEdgesFromObs(obs, tx, nil) - if len(edges) != 1 { - t.Fatalf("expected 1 edge for zero-hop advert, got %d", len(edges)) - } - // Canonical ordering: aaaa < bbbb - if edges[0].A != "aaaa1111" || edges[0].B != "bbbb2222" { - t.Errorf("unexpected edge: %+v", edges[0]) - } -} - -func TestExtractEdgesFromObs_NonAdvertNoPath(t *testing.T) { - tx := &StoreTx{PayloadType: intPtr(1)} - obs := &StoreObs{ObserverID: "obs1", PathJSON: ""} - edges := extractEdgesFromObs(obs, tx, nil) - if len(edges) != 0 { - t.Errorf("expected 0 edges for non-advert without path, got %d", len(edges)) - } -} - -func TestExtractEdgesFromObs_WithPath(t *testing.T) { - nodes := []nodeInfo{ - {Role: "repeater", PublicKey: "aabbccddee1234567890aabbccddee1234567890aabbccddee1234567890aabb", Name: "Node-AA"}, - {Role: "repeater", PublicKey: "ffgghhii1234567890aabbccddee1234567890aabbccddee1234567890aabb11", Name: "Node-FF"}, - } - pm := buildPrefixMap(nodes) - - tx := &StoreTx{ - DecodedJSON: `{"pubKey":"originator00"}`, - PayloadType: intPtr(4), - } - obs := &StoreObs{ - ObserverID: "observer00", - PathJSON: `["aa","ff"]`, - Timestamp: "2024-01-01T00:00:00Z", - } - - edges := extractEdgesFromObs(obs, tx, pm) - // Should get: originator↔aa (advert), observer↔ff (last hop) - if len(edges) != 2 { - t.Fatalf("expected 2 edges, got %d", len(edges)) - } -} - -func TestExtractEdgesFromObs_SameNodeNoEdge(t *testing.T) { - tx := &StoreTx{ - DecodedJSON: `{"pubKey":"same1234"}`, - PayloadType: intPtr(4), - } - obs := &StoreObs{ - ObserverID: "same1234", - PathJSON: "", - Timestamp: "2024-01-01T00:00:00Z", - } - edges := extractEdgesFromObs(obs, tx, nil) - if len(edges) != 0 { - t.Errorf("expected 0 edges when originator == observer, got %d", len(edges)) - } -} - - - -func TestPersistSemaphoreTryAcquireSkipsBatch(t *testing.T) { - // Verify that persistSem is a buffered channel of size 1. - if cap(persistSem) != 1 { - t.Errorf("persistSem capacity = %d, want 1", cap(persistSem)) - } - // Acquire the semaphore to simulate an in-progress persistence. - persistSem <- struct{}{} - - // asyncPersistResolvedPathsAndEdges should skip (not block, not - // spawn a goroutine) when the semaphore is already held. - done := make(chan struct{}) - go func() { - asyncPersistResolvedPathsAndEdges( - "/nonexistent/path.db", - []persistObsUpdate{{obsID: 1, resolvedPath: "x"}}, - nil, - "test", - ) - close(done) - }() - - // If the function blocks on the semaphore instead of skipping, - // this select will hit the timeout. - select { - case <-done: - // Expected: returned immediately because semaphore was busy. - case <-time.After(500 * time.Millisecond): - <-persistSem - t.Fatal("asyncPersistResolvedPathsAndEdges blocked instead of skipping when semaphore was held") - } - - <-persistSem // release -} - -func TestOpenRW_BusyTimeout(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - // Create the DB file first - db, err := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") - if err != nil { - t.Fatal(err) - } - db.Exec("CREATE TABLE dummy (id INTEGER)") - db.Close() - - // Open via openRW and verify busy_timeout is set - rw, err := openRW(dbPath) - if err != nil { - t.Fatalf("openRW failed: %v", err) - } - defer rw.Close() - - var timeout int - if err := rw.QueryRow("PRAGMA busy_timeout").Scan(&timeout); err != nil { - t.Fatalf("query busy_timeout: %v", err) - } - if timeout != 5000 { - t.Errorf("expected busy_timeout=5000, got %d", timeout) - } -} - -func TestEnsureLastPacketAtColumn(t *testing.T) { - // Create a temp DB with observers table missing last_packet_at - dir := t.TempDir() - dbPath := dir + "/test.db" - db, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - _, err = db.Exec(`CREATE TABLE observers ( - id TEXT PRIMARY KEY, - name TEXT, - last_seen TEXT, - lat REAL, - lon REAL, - inactive INTEGER DEFAULT 0 - )`) - if err != nil { - t.Fatal(err) - } - db.Close() - - // First call: should add the column - if err := ensureLastPacketAtColumn(dbPath); err != nil { - t.Fatalf("first call failed: %v", err) - } - - // Verify column exists - db2, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - defer db2.Close() - - var found bool - rows, err := db2.Query("PRAGMA table_info(observers)") - if err != nil { - t.Fatal(err) - } - defer rows.Close() - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "last_packet_at" { - found = true - } - } - if !found { - t.Fatal("last_packet_at column not found after migration") - } - - // Idempotency: second call should succeed without error - if err := ensureLastPacketAtColumn(dbPath); err != nil { - t.Fatalf("idempotent call failed: %v", err) - } -} - -// TestEnsureObserverIATAColumn validates the #1189 R1 fix: an operator with -// a pre-iata observers schema (no `iata TEXT` column) must not panic on -// startup. The migration must idempotently ALTER TABLE ADD COLUMN, and -// queries that COALESCE(obs.iata, '') must succeed after the migration runs. -func TestEnsureObserverIATAColumn(t *testing.T) { - dir := t.TempDir() - dbPath := dir + "/test.db" - - // Pre-iata schema (matches what shipped before #1188 landed). - db, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - _, err = db.Exec(`CREATE TABLE observers ( - id TEXT PRIMARY KEY, - name TEXT, - last_seen TEXT, - first_seen TEXT, - packet_count INTEGER DEFAULT 0, - inactive INTEGER DEFAULT 0, - last_packet_at TEXT DEFAULT NULL - )`) - if err != nil { - t.Fatal(err) - } - if _, err := db.Exec(`INSERT INTO observers (id, name) VALUES ('obs1', 'Observer One')`); err != nil { - t.Fatal(err) - } - db.Close() - - // Prove the bug exists pre-migration: a SELECT that COALESCEs obs.iata must fail. - db0, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - var probe string - preErr := db0.QueryRow(`SELECT COALESCE(iata, '') FROM observers WHERE id='obs1'`).Scan(&probe) - db0.Close() - if preErr == nil { - t.Fatal("expected SELECT on missing iata column to fail BEFORE migration; got success") - } - - // First call: should add the column. - if err := ensureObserverIATAColumn(dbPath); err != nil { - t.Fatalf("first call failed: %v", err) - } - - // Verify column exists. - db2, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatal(err) - } - defer db2.Close() - - var found bool - rows, err := db2.Query("PRAGMA table_info(observers)") - if err != nil { - t.Fatal(err) - } - for rows.Next() { - var cid int - var colName string - var colType sql.NullString - var notNull, pk int - var dflt sql.NullString - if rows.Scan(&cid, &colName, &colType, ¬Null, &dflt, &pk) == nil && colName == "iata" { - found = true - } - } - rows.Close() - if !found { - t.Fatal("iata column not found after migration") - } - - // The query that previously panicked must now succeed (empty string default). - if err := db2.QueryRow(`SELECT COALESCE(iata, '') FROM observers WHERE id='obs1'`).Scan(&probe); err != nil { - t.Fatalf("post-migration SELECT failed: %v", err) - } - if probe != "" { - t.Fatalf("expected empty iata for legacy row, got %q", probe) - } - - // Idempotency: second call must succeed. - if err := ensureObserverIATAColumn(dbPath); err != nil { - t.Fatalf("idempotent call failed: %v", err) - } -} diff --git a/cmd/server/neighbor_recomputer.go b/cmd/server/neighbor_recomputer.go new file mode 100644 index 00000000..41b33115 --- /dev/null +++ b/cmd/server/neighbor_recomputer.go @@ -0,0 +1,97 @@ +// Package main: neighbor-graph snapshot recomputer (issue #1287). +// +// Per #1287 Option 4: the ingestor owns the neighbor_edges table — +// it computes the graph from observations it ingests and persists +// snapshots there. The server READS the snapshot and atomic-swaps +// it into s.graph; that swap is exactly what this recomputer does. +// +// Cadence: 60s default. Staleness budget matches the existing +// analytics recomputer (#1240) — operators already accept that +// derived analytics lag the wire by tens of seconds. +package main + +import ( + "sync" + "time" +) + +// NeighborGraphRecomputerDefaultInterval is how often the server +// re-reads the neighbor_edges snapshot. 60s is the standard +// staleness budget for derived analytics (#1240 / #1262 / #672 axis 2). +const NeighborGraphRecomputerDefaultInterval = 60 * time.Second + +var ( + neighborRecompStartedMu sync.Mutex + neighborRecompStarted bool +) + +// StartNeighborGraphRecomputer launches the background goroutine that +// re-reads neighbor_edges every `interval` and atomic-swaps the +// resulting NeighborGraph into s.graph. Idempotent — subsequent calls +// are no-ops and return a no-op stop closure. +// +// Server NEVER writes to neighbor_edges; the ingestor owns those +// writes per #1287. This recomputer is the ONLY thing that updates +// s.graph at steady state (the initial startup load in main.go is the +// other writer to s.graph, only at boot). +func (s *PacketStore) StartNeighborGraphRecomputer(interval time.Duration) func() { + if interval <= 0 { + interval = NeighborGraphRecomputerDefaultInterval + } + + neighborRecompStartedMu.Lock() + if neighborRecompStarted { + neighborRecompStartedMu.Unlock() + return func() {} + } + neighborRecompStarted = true + stop := make(chan struct{}) + done := make(chan struct{}) + neighborRecompStartedMu.Unlock() + + var stopOnce sync.Once + go func() { + defer close(done) + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-t.C: + s.refreshNeighborGraphFromSnapshot() + case <-stop: + return + } + } + }() + + return func() { + stopOnce.Do(func() { close(stop) }) + select { + case <-done: + case <-time.After(5 * time.Second): + } + } +} + +// refreshNeighborGraphFromSnapshot re-reads neighbor_edges through +// the read-only DB handle and atomic-swaps a freshly built graph. +// Panics are swallowed defensively — the previous snapshot remains +// valid if a read fails. +func (s *PacketStore) refreshNeighborGraphFromSnapshot() { + defer func() { _ = recover() }() + if s.db == nil || s.db.conn == nil { + return + } + g := loadNeighborEdgesFromDB(s.db.conn) + if g != nil { + s.graph.Store(g) + } +} + +// resetNeighborRecomputerForTest is a test helper — production code +// MUST NOT call this. +func resetNeighborRecomputerForTest() { + neighborRecompStartedMu.Lock() + neighborRecompStarted = false + neighborRecompStartedMu.Unlock() +} diff --git a/cmd/server/neighbor_recomputer_test.go b/cmd/server/neighbor_recomputer_test.go new file mode 100644 index 00000000..f78770a4 --- /dev/null +++ b/cmd/server/neighbor_recomputer_test.go @@ -0,0 +1,132 @@ +package main + +import ( + "database/sql" + "path/filepath" + "testing" + "time" + + "github.com/meshcore-analyzer/dbschema" + _ "modernc.org/sqlite" +) + +// TestNeighborGraphRecomputerLoadsSnapshot enforces #1287 Option 4: +// the server LOADS its in-memory neighbor graph from the SQLite +// snapshot the ingestor writes. After a write to neighbor_edges (here +// done synthetically), the recomputer's atomic-swap must reflect it. +func TestNeighborGraphRecomputerLoadsSnapshot(t *testing.T) { + dir := t.TempDir() + dbPath := filepath.Join(dir, "neighbor_recomp.db") + + // Bootstrap a WAL DB with the neighbor_edges table. + rw, err := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") + if err != nil { + t.Fatal(err) + } + defer rw.Close() + if _, err := rw.Exec(`CREATE TABLE neighbor_edges ( + node_a TEXT NOT NULL, + node_b TEXT NOT NULL, + count INTEGER DEFAULT 1, + last_seen TEXT, + PRIMARY KEY (node_a, node_b) + )`); err != nil { + t.Fatal(err) + } + + // Stage one edge. + now := time.Now().UTC().Format(time.RFC3339) + if _, err := rw.Exec( + `INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) VALUES (?, ?, ?, ?)`, + "aaa", "bbb", 5, now, + ); err != nil { + t.Fatal(err) + } + + // Server opens read-only and refreshes via the recomputer. + d, err := OpenDB(dbPath) + if err != nil { + t.Fatalf("OpenDB: %v", err) + } + defer d.conn.Close() + store := &PacketStore{db: d} + store.graph.Store(NewNeighborGraph()) + + store.refreshNeighborGraphFromSnapshot() + g := store.graph.Load() + if g == nil { + t.Fatal("graph nil after refresh") + } + if got := len(g.AllEdges()); got != 1 { + t.Fatalf("expected 1 edge after first refresh, got %d", got) + } + + // Add another row, refresh, assert the new total. + if _, err := rw.Exec( + `INSERT INTO neighbor_edges (node_a, node_b, count, last_seen) VALUES (?, ?, ?, ?)`, + "ccc", "ddd", 2, now, + ); err != nil { + t.Fatal(err) + } + store.refreshNeighborGraphFromSnapshot() + g = store.graph.Load() + if got := len(g.AllEdges()); got != 2 { + t.Fatalf("expected 2 edges after second refresh, got %d", got) + } +} + +// TestServerStartupRequiresMigratedSchema enforces #1287: the server +// MUST refuse to start if the ingestor hasn't run schema migrations. +// AssertReady on a DB missing the required columns returns an error +// listing every missing surface; main.go then calls log.Fatalf. +func TestServerStartupRequiresMigratedSchema(t *testing.T) { + dir := t.TempDir() + dbPath := filepath.Join(dir, "unmigrated.db") + + // Bootstrap with ONLY transmissions/observations (the things + // server tries to read) but WITHOUT the columns dbschema asserts + // (resolved_path, inactive, last_packet_at, iata, foreign_advert, + // from_pubkey, neighbor_edges). + rw, err := sql.Open("sqlite", "file:"+dbPath+"?_journal_mode=WAL") + if err != nil { + t.Fatal(err) + } + defer rw.Close() + for _, s := range []string{ + `CREATE TABLE transmissions (id INTEGER PRIMARY KEY, hash TEXT, payload_type INTEGER)`, + `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER)`, + `CREATE TABLE observers (id TEXT PRIMARY KEY, name TEXT)`, + `CREATE TABLE nodes (public_key TEXT PRIMARY KEY)`, + `CREATE TABLE inactive_nodes (public_key TEXT PRIMARY KEY)`, + } { + if _, err := rw.Exec(s); err != nil { + t.Fatal(err) + } + } + + // Open the read-only server handle and call AssertReady directly + // (production path: main.go does this before any business logic). + d, err := OpenDB(dbPath) + if err != nil { + t.Fatalf("OpenDB: %v", err) + } + defer d.conn.Close() + + // The package-level dbschema.AssertReady requires every missing + // surface to be reported. We hit it directly through the same + // path main.go uses. + if err := assertReadyForTest(d); err == nil { + t.Fatal("expected AssertReady to fail against an unmigrated DB; server would have started against an incomplete schema") + } +} + +// assertReadyForTest is the same call main.go makes — declared here so +// the test stays decoupled from any future inlining or rename. +func assertReadyForTest(d *DB) error { + return dbschemaAssertReadyShim(d) +} + +// dbschemaAssertReadyShim wraps the package import so tests don't +// directly depend on the import being present (production wires it +// via main.go). +func dbschemaAssertReadyShim(d *DB) error { return dbschema.AssertReady(d.conn) } diff --git a/cmd/server/readonly_invariant_test.go b/cmd/server/readonly_invariant_test.go index 629009d8..33ceabab 100644 --- a/cmd/server/readonly_invariant_test.go +++ b/cmd/server/readonly_invariant_test.go @@ -3,12 +3,63 @@ package main import ( "database/sql" "fmt" + "os" + "path/filepath" "reflect" + "regexp" + "strings" "testing" _ "modernc.org/sqlite" ) +// TestServerSourceHasNoCachedRWCalls enforces issue #1287: after the +// follow-up to #1283, cmd/server/ must contain ZERO writer call sites. +// Specifically, no `cachedRW(`, no `mode=rw`, and no `sql.Open(...rw...)` +// in non-test source files. All schema migrations, backfills, and +// neighbor-edge persistence must live in cmd/ingestor or a shared +// package — the server is the read path. +func TestServerSourceHasNoCachedRWCalls(t *testing.T) { + entries, err := os.ReadDir(".") + if err != nil { + t.Fatalf("read cmd/server dir: %v", err) + } + // Patterns that indicate write-side DB usage on the server. + patterns := []*regexp.Regexp{ + regexp.MustCompile(`\bcachedRW\s*\(`), + regexp.MustCompile(`mode=rw`), + regexp.MustCompile(`sql\.Open\([^)]*\?[^)]*_journal_mode=WAL[^)]*\)`), + } + violations := []string{} + for _, e := range entries { + name := e.Name() + if e.IsDir() { + continue + } + if !strings.HasSuffix(name, ".go") { + continue + } + if strings.HasSuffix(name, "_test.go") { + continue + } + b, err := os.ReadFile(filepath.Join(".", name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + for _, p := range patterns { + if loc := p.FindIndex(b); loc != nil { + // Get line number + line := 1 + strings.Count(string(b[:loc[0]]), "\n") + violations = append(violations, fmt.Sprintf("%s:%d: %s", name, line, p.String())) + } + } + } + if len(violations) > 0 { + t.Errorf("cmd/server/ contains forbidden writer call sites (#1287):\n %s", + strings.Join(violations, "\n ")) + } +} + // TestServerDBHasNoWriteMethods enforces the architectural invariant from // issue #1283: cmd/server is the read path. All write/maintenance methods // (PruneOldPackets, PruneOldMetrics, RemoveStaleObservers) MUST live on diff --git a/cmd/server/rw_cache.go b/cmd/server/rw_cache.go deleted file mode 100644 index b22fa11d..00000000 --- a/cmd/server/rw_cache.go +++ /dev/null @@ -1,59 +0,0 @@ -package main - -import ( - "database/sql" - "fmt" - "sync" -) - -// rwCache holds a process-wide cached RW connection per database path. -// Instead of opening and closing a new RW connection on every call to openRW, -// we cache a single *sql.DB (which internally manages one connection due to -// SetMaxOpenConns(1)). This eliminates repeated open/close overhead for -// vacuum, prune, persist operations that run frequently (#921). -var rwCache = struct { - mu sync.Mutex - conns map[string]*sql.DB -}{conns: make(map[string]*sql.DB)} - -// cachedRW returns a cached read-write connection for the given dbPath. -// The connection is created on first call and reused thereafter. -// Callers MUST NOT call Close() on the returned *sql.DB. -func cachedRW(dbPath string) (*sql.DB, error) { - rwCache.mu.Lock() - defer rwCache.mu.Unlock() - - if db, ok := rwCache.conns[dbPath]; ok { - return db, nil - } - - dsn := fmt.Sprintf("file:%s?_journal_mode=WAL", dbPath) - db, err := sql.Open("sqlite", dsn) - if err != nil { - return nil, err - } - db.SetMaxOpenConns(1) - if _, err := db.Exec("PRAGMA busy_timeout = 5000"); err != nil { - db.Close() - return nil, fmt.Errorf("set busy_timeout: %w", err) - } - rwCache.conns[dbPath] = db - return db, nil -} - -// closeRWCache closes all cached RW connections (for tests/shutdown). -func closeRWCache() { - rwCache.mu.Lock() - defer rwCache.mu.Unlock() - for k, db := range rwCache.conns { - db.Close() - delete(rwCache.conns, k) - } -} - -// rwCacheLen returns the number of cached connections (for testing). -func rwCacheLen() int { - rwCache.mu.Lock() - defer rwCache.mu.Unlock() - return len(rwCache.conns) -} diff --git a/cmd/server/rw_cache_test.go b/cmd/server/rw_cache_test.go deleted file mode 100644 index 96c76369..00000000 --- a/cmd/server/rw_cache_test.go +++ /dev/null @@ -1,55 +0,0 @@ -package main - -import ( - "os" - "path/filepath" - "testing" -) - -func TestCachedRW_ReturnsSameHandle(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - - // Create the DB file - f, _ := os.Create(dbPath) - f.Close() - - defer closeRWCache() - - db1, err := cachedRW(dbPath) - if err != nil { - t.Fatalf("first cachedRW: %v", err) - } - db2, err := cachedRW(dbPath) - if err != nil { - t.Fatalf("second cachedRW: %v", err) - } - if db1 != db2 { - t.Fatalf("cachedRW returned different handles: %p vs %p", db1, db2) - } -} - -func TestCachedRW_100Calls_SingleConnection(t *testing.T) { - dir := t.TempDir() - dbPath := filepath.Join(dir, "test.db") - f, _ := os.Create(dbPath) - f.Close() - - defer closeRWCache() - - var first interface{} - for i := 0; i < 100; i++ { - db, err := cachedRW(dbPath) - if err != nil { - t.Fatalf("call %d: %v", i, err) - } - if i == 0 { - first = db - } else if db != first { - t.Fatalf("call %d returned different handle", i) - } - } - if rwCacheLen() != 1 { - t.Fatalf("expected 1 cached connection, got %d", rwCacheLen()) - } -} diff --git a/cmd/server/store.go b/cmd/server/store.go index e6292aff..c0d16915 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -2380,41 +2380,13 @@ func (s *PacketStore) IngestNewFromDB(sinceID, limit int) ([]map[string]interfac s.invalidateCachesFor(inv) } - // Persist resolved paths and neighbor edges asynchronously (don't block ingest). - if len(broadcastTxs) > 0 && s.db != nil { - dbPath := s.db.path - var obsUpdates []persistObsUpdate - var edgeUpdates []persistEdgeUpdate - - _, pm := s.getCachedNodesAndPM() - // graph is *atomic.Pointer[NeighborGraph]; the Load itself is - // lock-free. (Earlier comment claimed "set during startup, not - // replaced after" — that's no longer true: #1203 made rebuilds - // async via ensureNeighborGraph. Dropping the dead s.mu RLock - // wrap — review PR #1208.) - graphRef := s.graph.Load() - for _, tx := range broadcastTxs { - for _, obs := range tx.Observations { - // Use decode-window resolved path for persist - if broadcastRP != nil { - if rp, ok := broadcastRP[obs.ID]; ok && rp != nil { - rpJSON := marshalResolvedPath(rp) - if rpJSON != "" { - obsUpdates = append(obsUpdates, persistObsUpdate{obs.ID, rpJSON}) - } - } - } - for _, ec := range extractEdgesFromObs(obs, tx, pm) { - edgeUpdates = append(edgeUpdates, persistEdgeUpdate{ec.A, ec.B, ec.Timestamp}) - if graphRef != nil { - graphRef.upsertEdge(ec.A, ec.B, "", obs.ObserverID, obs.SNR, parseTimestamp(ec.Timestamp)) - } - } - } - } - - asyncPersistResolvedPathsAndEdges(dbPath, obsUpdates, edgeUpdates, "persist") - } + // Per #1287 (Option 4): the server NEVER writes to the DB and + // NEVER mutates the in-memory neighbor graph incrementally. The + // ingestor owns neighbor_edges; recompNeighborGraph re-reads the + // snapshot every 60s and atomic-swaps it into s.graph. We also no + // longer persist resolved_path here — the ingestor (which already + // sees every observation) owns that write too. + _ = broadcastRP // resolved path is still computed in-memory (above) for live broadcast; no SQL write. return result, newMaxID } @@ -2727,38 +2699,14 @@ func (s *PacketStore) IngestNewObservations(sinceObsID, limit int) []map[string] }) } - // Persist resolved paths and neighbor edges asynchronously (review fix #3). - // Only process NEW observations — not all observations of each updated tx — - // to avoid edge count inflation and unnecessary UPDATEs for pre-existing data. - if len(newObs) > 0 && s.db != nil { - dbPath := s.db.path - var obsUpdates []persistObsUpdate - var edgeUpdates []persistEdgeUpdate - - for _, obs := range newObs { - tx := s.byTxID[obs.TransmissionID] - if tx == nil { - continue - } - // Use decode-window resolved path for persist - if obsRPMap != nil { - if rp, ok := obsRPMap[obs.ID]; ok && rp != nil { - rpJSON := marshalResolvedPath(rp) - if rpJSON != "" { - obsUpdates = append(obsUpdates, persistObsUpdate{obs.ID, rpJSON}) - } - } - } - for _, ec := range extractEdgesFromObs(obs, tx, pm) { - edgeUpdates = append(edgeUpdates, persistEdgeUpdate{ec.A, ec.B, ec.Timestamp}) - if graphRef != nil { - graphRef.upsertEdge(ec.A, ec.B, "", obs.ObserverID, obs.SNR, parseTimestamp(ec.Timestamp)) - } - } - } - - asyncPersistResolvedPathsAndEdges(dbPath, obsUpdates, edgeUpdates, "obs-persist") - } + // Per #1287 (Option 4): server never writes to the DB and never + // mutates the in-memory neighbor graph incrementally — the + // ingestor owns both. recompNeighborGraph re-reads the snapshot + // every 60s and atomic-swaps into s.graph. + _ = obsRPMap // resolved path stays in-memory for broadcast; no SQL write. + _ = newObs + _ = pm + _ = graphRef return broadcastMaps } diff --git a/internal/dbschema/dbschema.go b/internal/dbschema/dbschema.go new file mode 100644 index 00000000..7e856c76 --- /dev/null +++ b/internal/dbschema/dbschema.go @@ -0,0 +1,326 @@ +// Package dbschema centralizes schema migrations and read-side schema +// assertions for the CoreScope SQLite DB. Per issue #1287 the writer +// (cmd/ingestor) owns ALL CREATE/ALTER/INSERT/UPDATE/DELETE on schema +// objects; the server (cmd/server) only ASSERTS that the schema is in +// the expected shape and refuses to start otherwise. +// +// Apply(rw, log) runs from the ingestor at startup BEFORE subscribing to +// MQTT. AssertReady(ro) runs from the server at startup and returns an +// error listing every missing column/index/table. +package dbschema + +import ( + "database/sql" + "errors" + "fmt" + "strings" +) + +// Logger is the minimal logging surface used by Apply. Both cmd/server +// and cmd/ingestor satisfy this with the stdlib `log` package's Printf +// (passed as a closure to avoid an indirect log dependency here). +type Logger func(format string, args ...interface{}) + +// Apply runs every server-side ensure_* migration against the given +// read-write SQLite connection. Each operation is idempotent +// (IF NOT EXISTS / column-probe-before-ALTER). Safe to call repeatedly. +// +// Called by the ingestor at startup. The server MUST NOT call this — +// it only calls AssertReady. +func Apply(rw *sql.DB, logf Logger) error { + if logf == nil { + logf = func(string, ...interface{}) {} + } + if err := ensureServerIndexes(rw); err != nil { + return fmt.Errorf("ensure server indexes: %w", err) + } + if err := ensureNeighborEdgesTable(rw); err != nil { + return fmt.Errorf("ensure neighbor_edges: %w", err) + } + if err := ensureInactiveNodesTable(rw); err != nil { + return fmt.Errorf("ensure inactive_nodes: %w", err) + } + if err := ensureResolvedPathColumn(rw, logf); err != nil { + return fmt.Errorf("ensure resolved_path: %w", err) + } + if err := ensureObserverInactiveColumn(rw, logf); err != nil { + return fmt.Errorf("ensure observers.inactive: %w", err) + } + if err := ensureLastPacketAtColumn(rw, logf); err != nil { + return fmt.Errorf("ensure observers.last_packet_at: %w", err) + } + if err := ensureObserverIATAColumn(rw, logf); err != nil { + return fmt.Errorf("ensure observers.iata: %w", err) + } + if err := ensureForeignAdvertColumn(rw, logf); err != nil { + return fmt.Errorf("ensure foreign_advert: %w", err) + } + if err := ensureFromPubkeyColumn(rw, logf); err != nil { + return fmt.Errorf("ensure from_pubkey: %w", err) + } + return nil +} + +// AssertReady verifies the schema is in the expected shape. The server +// calls this at startup against a read-only connection; if it returns +// non-nil, the server MUST fatal-log and exit so the operator restarts +// the ingestor (which owns migrations). +func AssertReady(ro *sql.DB) error { + var missing []string + + mustCol := func(table, col string) { + has, err := TableHasColumn(ro, table, col) + if err != nil { + missing = append(missing, fmt.Sprintf("%s.%s (probe error: %v)", table, col, err)) + return + } + if !has { + missing = append(missing, fmt.Sprintf("%s.%s", table, col)) + } + } + mustTable := func(name string) { + var n int + err := ro.QueryRow(`SELECT 1 FROM sqlite_master WHERE type='table' AND name=?`, name).Scan(&n) + if errors.Is(err, sql.ErrNoRows) { + missing = append(missing, "table:"+name) + } else if err != nil { + missing = append(missing, fmt.Sprintf("table:%s (probe error: %v)", name, err)) + } + } + + mustTable("neighbor_edges") + mustCol("observations", "resolved_path") + mustCol("observers", "inactive") + mustCol("observers", "last_packet_at") + mustCol("observers", "iata") + mustCol("nodes", "foreign_advert") + mustCol("inactive_nodes", "foreign_advert") + mustCol("transmissions", "from_pubkey") + + if len(missing) > 0 { + return fmt.Errorf("schema not migrated by ingestor; restart ingestor first. missing: %s", + strings.Join(missing, ", ")) + } + return nil +} + +// TableHasColumn reports whether the given table has the given column. +// Exported because tests and the read-side need it without re-implementing. +func TableHasColumn(db *sql.DB, table, column string) (bool, error) { + rows, err := db.Query(fmt.Sprintf("PRAGMA table_info(%s)", table)) + if err != nil { + return false, err + } + defer rows.Close() + for rows.Next() { + var cid int + var name string + var ctype sql.NullString + var notnull, pk int + var dflt sql.NullString + if err := rows.Scan(&cid, &name, &ctype, ¬null, &dflt, &pk); err != nil { + return false, err + } + if name == column { + return true, nil + } + } + return false, rows.Err() +} + +// ─── ensure_* helpers (writer side) ──────────────────────────────────────── + +func ensureServerIndexes(rw *sql.DB) error { + stmts := []string{ + `CREATE INDEX IF NOT EXISTS idx_transmissions_first_seen ON transmissions(first_seen)`, + `CREATE INDEX IF NOT EXISTS idx_transmissions_hash ON transmissions(hash)`, + `CREATE INDEX IF NOT EXISTS idx_transmissions_payload_type ON transmissions(payload_type)`, + `CREATE INDEX IF NOT EXISTS idx_observations_timestamp ON observations(timestamp)`, + `CREATE INDEX IF NOT EXISTS idx_observations_transmission_id ON observations(transmission_id)`, + } + for _, s := range stmts { + if _, err := rw.Exec(s); err != nil { + return fmt.Errorf("ensure index %q: %w", s, err) + } + } + // observer_idx (v3) vs observer_id (v2) — probe + index the matching one. + hasIdx, err := TableHasColumn(rw, "observations", "observer_idx") + if err != nil { + return err + } + if hasIdx { + if _, err := rw.Exec(`CREATE INDEX IF NOT EXISTS idx_observations_observer_idx ON observations(observer_idx)`); err != nil { + return err + } + } + hasID, err := TableHasColumn(rw, "observations", "observer_id") + if err != nil { + return err + } + if hasID { + if _, err := rw.Exec(`CREATE INDEX IF NOT EXISTS idx_observations_observer_id ON observations(observer_id)`); err != nil { + return err + } + } + return nil +} + +func ensureNeighborEdgesTable(rw *sql.DB) error { + _, err := rw.Exec(`CREATE TABLE IF NOT EXISTS neighbor_edges ( + node_a TEXT NOT NULL, + node_b TEXT NOT NULL, + count INTEGER DEFAULT 1, + last_seen TEXT, + PRIMARY KEY (node_a, node_b) + )`) + return err +} + +// ensureInactiveNodesTable creates the inactive_nodes table if missing. +// The ingestor's applySchema also creates this table — duplicating it +// here makes dbschema.Apply self-sufficient when called against a +// fixture DB that pre-dates the soft-delete feature (e.g. CI's +// test-fixtures/e2e-fixture.db, which never had any inactive rows). +// Schema kept in sync with cmd/ingestor/db.go:applySchema. +func ensureInactiveNodesTable(rw *sql.DB) error { + _, err := rw.Exec(`CREATE TABLE IF NOT EXISTS inactive_nodes ( + public_key TEXT PRIMARY KEY, + name TEXT, + role TEXT, + lat REAL, + lon REAL, + last_seen TEXT, + first_seen TEXT, + advert_count INTEGER DEFAULT 0, + battery_mv INTEGER, + temperature_c REAL, + foreign_advert INTEGER DEFAULT 0 + )`) + if err != nil { + return err + } + _, err = rw.Exec(`CREATE INDEX IF NOT EXISTS idx_inactive_nodes_last_seen ON inactive_nodes(last_seen)`) + return err +} + +func ensureResolvedPathColumn(rw *sql.DB, logf Logger) error { + has, err := TableHasColumn(rw, "observations", "resolved_path") + if err != nil { + return err + } + if has { + return nil + } + if _, err := rw.Exec("ALTER TABLE observations ADD COLUMN resolved_path TEXT"); err != nil { + return err + } + logf("[dbschema] added resolved_path column to observations") + return nil +} + +func ensureObserverInactiveColumn(rw *sql.DB, logf Logger) error { + has, err := TableHasColumn(rw, "observers", "inactive") + if err != nil { + return err + } + if has { + return nil + } + if _, err := rw.Exec("ALTER TABLE observers ADD COLUMN inactive INTEGER DEFAULT 0"); err != nil { + return err + } + logf("[dbschema] added inactive column to observers") + return nil +} + +func ensureLastPacketAtColumn(rw *sql.DB, logf Logger) error { + has, err := TableHasColumn(rw, "observers", "last_packet_at") + if err != nil { + return err + } + if has { + return nil + } + if _, err := rw.Exec("ALTER TABLE observers ADD COLUMN last_packet_at TEXT"); err != nil { + return err + } + logf("[dbschema] added last_packet_at column to observers") + return nil +} + +func ensureObserverIATAColumn(rw *sql.DB, logf Logger) error { + has, err := TableHasColumn(rw, "observers", "iata") + if err != nil { + return err + } + if has { + return nil + } + if _, err := rw.Exec("ALTER TABLE observers ADD COLUMN iata TEXT"); err != nil { + return err + } + logf("[dbschema] added iata column to observers") + return nil +} + +func ensureForeignAdvertColumn(rw *sql.DB, logf Logger) error { + for _, table := range []string{"nodes", "inactive_nodes"} { + has, err := TableHasColumn(rw, table, "foreign_advert") + if err != nil { + return fmt.Errorf("inspect %s: %w", table, err) + } + if has { + continue + } + if _, err := rw.Exec(fmt.Sprintf("ALTER TABLE %s ADD COLUMN foreign_advert INTEGER DEFAULT 0", table)); err != nil { + return err + } + logf("[dbschema] added foreign_advert column to %s", table) + } + return nil +} + +func ensureFromPubkeyColumn(rw *sql.DB, logf Logger) error { + has, err := TableHasColumn(rw, "transmissions", "from_pubkey") + if err != nil { + return err + } + if !has { + if _, err := rw.Exec("ALTER TABLE transmissions ADD COLUMN from_pubkey TEXT"); err != nil { + return err + } + logf("[dbschema] added from_pubkey column to transmissions (#1143)") + } + if _, err := rw.Exec("CREATE INDEX IF NOT EXISTS idx_transmissions_from_pubkey ON transmissions(from_pubkey)"); err != nil { + return err + } + return nil +} + +// SoftDeleteBlacklistedObservers marks the given observer IDs as +// inactive=1 (case-insensitive match). Returns count affected. +// Writer-side helper; ingestor calls it at startup with the operator +// blacklist (read from config). +func SoftDeleteBlacklistedObservers(rw *sql.DB, blacklist []string) (int64, error) { + placeholders := make([]string, 0, len(blacklist)) + args := make([]interface{}, 0, len(blacklist)) + for _, pk := range blacklist { + t := strings.TrimSpace(pk) + if t == "" { + continue + } + placeholders = append(placeholders, "LOWER(?)") + args = append(args, t) + } + if len(placeholders) == 0 { + return 0, nil + } + q := "UPDATE observers SET inactive = 1 WHERE LOWER(id) IN (" + + strings.Join(placeholders, ",") + ") AND (inactive IS NULL OR inactive = 0)" + res, err := rw.Exec(q, args...) + if err != nil { + return 0, err + } + n, _ := res.RowsAffected() + return n, nil +} diff --git a/internal/dbschema/go.mod b/internal/dbschema/go.mod new file mode 100644 index 00000000..3c48ea7f --- /dev/null +++ b/internal/dbschema/go.mod @@ -0,0 +1,3 @@ +module github.com/meshcore-analyzer/dbschema + +go 1.22