profit/golangLAKEHOUSE
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

G1P: vectord persistence to storaged + scrum (3 fixes incl. 3-way convergent)

Browse Source 
Adds optional persistence to vectord (G1's HNSW vector search). Single-
file framed format per index — eliminates the torn-write class that
the 3-way convergent scrum finding identified:

  _vectors/<name>.lhv1  — single binary blob:
      [4 bytes magic "LHV1"]
      [4 bytes envelope_len uint32 BE]
      [envelope bytes — JSON params + metadata + version]
      [graph bytes — raw hnsw.Graph.Export]

Pre-extraction: internal/catalogd/store_client.go → internal/storeclient/
shared package, since both catalogd and vectord need it. Same pattern as
the pre-D5 catalogclient extraction.

Optional via [vectord].storaged_url config (empty = ephemeral mode).
On startup: List + Load each persisted index. After Create / batch Add /
DELETE: Save (or Delete from storaged). Save failures are logged-not-
fatal — in-memory state is the source of truth in flight.

Acceptance smoke G1P 8/8 PASS — kill+restart preserves state, post-
restart search returns dist=0 (graph round-trips exactly), DELETE
removes the file, post-delete restart shows count=0.

All 8 smokes (D1-D6 + G1 + G1P) PASS deterministically. The g1_smoke
gained scripts/g1_smoke.toml that disables persistence so the
in-memory API test stays decoupled from any rehydrate-from-storaged
state contamination.

Cross-lineage scrum on shipped code:
  - Opus 4.7 (opencode):                     1 BLOCK + 5 WARN + 3 INFO
  - Kimi K2-0905 (openrouter):               1 BLOCK + 2 WARN
  - Qwen3-coder (openrouter):                2 BLOCK + 2 WARN + 1 INFO

Fixed (3 — 1 convergent + 2 single-reviewer):
  C1 (Opus + Kimi + Qwen 3-WAY CONVERGENT WARN): Save was non-atomic
    across two PUTs — envelope-succeeds + graph-fails left a half-
    saved index that passed the "both present" List filter and
    silently mismatched metadata against vectors on Load.
    Fix: collapse to single framed file (no torn-write window
    possible).
  O-B1 (Opus BLOCK): isNotFound substring-matched "key not found"
    against the wrapped error message — brittle, any 5xx body
    containing that text would silently misclassify as missing.
    Fix: errors.Is(err, storeclient.ErrKeyNotFound).
  O-I3 (Opus INFO): handleAdd pre-validation only covered id+dim;
    NaN/Inf/zero-norm could still fail mid-batch leaving partial
    commits. Fix: extend pre-validation to call ValidateVector
    (newly exported) per item before any commit.

Dismissed (3 false positives):
  K-B1 + Q-B1 ("safeKey double-escapes %2F segments") — false
    convergent. Wire-protocol escape is decoded by storaged's chi
    router on the way in; on-disk key is the original literal.
    %2F round-trips correctly through PathEscape → URL → chi decode
    → S3 key.
  Q-B2 ("List vulnerable to race conditions") — vectord is single-
    process; no concurrent Save against List in the same vectord.

Deferred (3): rehydrate per-index timeout (G2+ multi-index scale),
saveAfter request ctx (matches G0 timeout deferral), Encode RLock
during slow writer (documented as buffer-only API).

The C1 finding is the strongest signal of the cross-lineage filter:
three independent reviewers all flagged the same torn-write hazard.
Single-file framing eliminates the class — there's now no Persistor
state where envelope and graph can disagree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
root
2026-04-29 01:33:23 -05:00
co-authored by Claude Opus 4.7
parent b8c072cf0b
commit 8b92518d21
14 changed files with 859 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/catalogd/registry_test.go
+3 -1
View File
@@ -6,6 +6,8 @@ import (
"sync"
"testing"
"time"
"git.agentview.dev/profit/golangLAKEHOUSE/internal/storeclient"
)
// memStore is an in-memory Store fake for unit tests.
@@ -30,7 +32,7 @@ func (m *memStore) Get(_ context.Context, key string) ([]byte, error) {
defer m.mu.Unlock()
b, ok := m.data[key]
if !ok {
return nil, ErrKeyNotFound
return nil, storeclient.ErrKeyNotFound
}
return b, nil
}
internal/catalogd/store_client.go
-140
View File
@@ -1,140 +0,0 @@
// store_client.go — thin HTTP client to storaged. catalogd needs to
// PUT manifest Parquets, GET them on startup, and LIST the manifests
// directory. Staying inside an HTTP boundary (rather than reaching
// into storaged's package directly) preserves the service-boundary
// shape that gRPC will eventually formalize at G1+.
package catalogd
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"time"
)
// StoreClient talks HTTP to the storaged service.
type StoreClient struct {
baseURL string
hc *http.Client
}
// listResponse mirrors storaged's GET /storage/list shape:
//
// {"prefix":"_catalog/manifests/","objects":[{Key,Size,ETag,LastModified}, ...]}
type listResponse struct {
Prefix string `json:"prefix"`
Objects []struct {
Key string `json:"Key"`
Size int64 `json:"Size"`
} `json:"objects"`
}
// NewStoreClient builds a client against the given storaged base URL
// (e.g. "http://127.0.0.1:3211"). Timeout covers manifest read-write
// only; rehydration of many manifests at startup runs sequentially
// and each call gets its own timeout window.
func NewStoreClient(baseURL string) *StoreClient {
return &StoreClient{
baseURL: strings.TrimRight(baseURL, "/"),
hc: &http.Client{Timeout: 30 * time.Second},
}
}
// Put writes raw bytes at key. body is the encoded Parquet manifest.
func (c *StoreClient) Put(ctx context.Context, key string, body []byte) error {
u := c.baseURL + "/storage/put/" + safeKey(key)
req, err := http.NewRequestWithContext(ctx, http.MethodPut, u, bytes.NewReader(body))
if err != nil {
return fmt.Errorf("put req: %w", err)
}
req.ContentLength = int64(len(body))
resp, err := c.hc.Do(req)
if err != nil {
return fmt.Errorf("put do: %w", err)
}
defer drainAndClose(resp.Body)
if resp.StatusCode != http.StatusOK {
preview, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return fmt.Errorf("put %s: status %d: %s", key, resp.StatusCode, string(preview))
}
return nil
}
// Get reads the bytes at key. ErrKeyNotFound on 404.
func (c *StoreClient) Get(ctx context.Context, key string) ([]byte, error) {
u := c.baseURL + "/storage/get/" + safeKey(key)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
if err != nil {
return nil, fmt.Errorf("get req: %w", err)
}
resp, err := c.hc.Do(req)
if err != nil {
return nil, fmt.Errorf("get do: %w", err)
}
defer drainAndClose(resp.Body)
if resp.StatusCode == http.StatusNotFound {
return nil, ErrKeyNotFound
}
if resp.StatusCode != http.StatusOK {
preview, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return nil, fmt.Errorf("get %s: status %d: %s", key, resp.StatusCode, string(preview))
}
return io.ReadAll(resp.Body)
}
// List returns the keys under prefix. Object metadata beyond Key is
// ignored — catalogd only needs the keys to drive rehydration.
func (c *StoreClient) List(ctx context.Context, prefix string) ([]string, error) {
u := c.baseURL + "/storage/list?prefix=" + url.QueryEscape(prefix)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
if err != nil {
return nil, fmt.Errorf("list req: %w", err)
}
resp, err := c.hc.Do(req)
if err != nil {
return nil, fmt.Errorf("list do: %w", err)
}
defer drainAndClose(resp.Body)
if resp.StatusCode != http.StatusOK {
preview, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return nil, fmt.Errorf("list %s: status %d: %s", prefix, resp.StatusCode, string(preview))
}
var lr listResponse
if err := json.NewDecoder(resp.Body).Decode(&lr); err != nil {
return nil, fmt.Errorf("list decode: %w", err)
}
out := make([]string, 0, len(lr.Objects))
for _, o := range lr.Objects {
out = append(out, o.Key)
}
return out, nil
}
// ErrKeyNotFound mirrors storaged's not-found semantics on the catalogd
// side without exposing storaged's package types.
var ErrKeyNotFound = fmt.Errorf("catalogd store: key not found")
// safeKey URL-escapes path segments while preserving "/". storaged's
// chi `/storage/<verb>/*` routes accept literal slashes in the
// wildcard match, so we only escape the segments, not the separators.
func safeKey(key string) string {
parts := strings.Split(key, "/")
for i, p := range parts {
parts[i] = url.PathEscape(p)
}
return strings.Join(parts, "/")
}
// drainAndClose reads any remaining body bytes (capped at 64 KiB) and
// closes the body. Per scrum S6 (Qwen): preview-then-close on error
// paths leaves bytes in the kernel buffer, breaking HTTP/1.1 keep-
// alive reuse and slowly leaking sockets.
func drainAndClose(body io.ReadCloser) {
_, _ = io.Copy(io.Discard, io.LimitReader(body, 64<<10))
_ = body.Close()
}