Repository navigation
Group commit, gRPC roles, /metrics, beastbench and security policy - #2
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…g guide Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Hand-written text exposition (no new deps). Authenticated by admin session or bearer token (--api-token / BEASTDB_METRICS_TOKEN); no loopback bypass. Adds buffer-pool hit/miss counters and checkpoint timestamps, gRPC metrics interceptors, runbook docs with scrape config and alert rules. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
WAL gains Append (assign LSN, no fsync), WaitDurable/Sync with a shared syncedLSN so one fsync covers all appended records, and a sticky fsync error. Write keeps its signature and durability but now group-commits too. Engine writes (Put/PutIfAbsent/PutIf/Delete/DeleteIf/BatchWrite[If] and replicated apply) go through a commit queue: a committer goroutine runs a group of writes under the engine lock (append + apply), issues one fsync, notifies the observer in LSN order, then releases the lock and acks writers. Reads and replication never see a write before it is durable. A failed fsync halts the engine and fails every write in the group. Concurrent Put benchmark: 7.82 ms/op -> 0.51 ms/op (~15x). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Group commit applies writes to pages before the group's fsync, so the buffer pool could flush a page ahead of its WAL records. Add a WAL barrier (WAL.EnsureDurable) called before eviction, FlushPage and FlushAll; a failing barrier blocks the page write. Adds regression tests and the upgrade spec. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Resolved conflicts in engine.go and buffer_pool.go by keeping both the group-commit/WAL-barrier fields and the metrics counters. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ench and Studio design Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… the 200-line limit No behaviour change: functions moved verbatim into wal_sync.go, wal_lifecycle.go, buffer_pool_wal.go, buffer_pool_stats.go, runner_load.go and runner_run.go. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…guideline About 400 lines is a prompt to ask whether a file does more than one thing, not a gate. Generated code, tests and data are exempt. Updates GEMINI.md (Antigravity), CONTRIBUTING.md and demo.ps1, and adds AGENTS.md (Codex) and CLAUDE.md (imports AGENTS.md). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6b851e682
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var chooser KeyChooser | ||
| switch { | ||
| case w.Latest: | ||
| chooser = Latest{Zipf: NewZipfian(cfg.Records, cfg.ZipfTheta), Frontier: frontier} |
There was a problem hiding this comment.
With workload D, all workers share this frontier: reads choose keys relative to its current value, while concurrent inserts advance it in worker. Scheduler interleaving therefore changes the per-worker key sequence even with the same seed, thread count, and operation count, undermining the benchmarking guide’s reproducibility claim. Use deterministic per-worker key assignment or qualify that guarantee.
Useful? React with 👍 / 👎.
| rep := Report{Tool: "beastbench", GoVersion: runtime.Version(), GOOS: runtime.GOOS, GOARCH: runtime.GOARCH, | ||
| NumCPU: runtime.NumCPU(), SelfHosted: *self, Addr: co.Addr, Config: cfg} |
There was a problem hiding this comment.
When --conns varies, Dial opens that many connections and workers distribute requests across them, but the report records only Config, which does not include the connection count. JSON results from runs with materially different connection settings therefore omit a throughput-relevant parameter and cannot reproduce or fairly compare those runs; include the effective count in the report.
Useful? React with 👍 / 👎.
…erformance-3d83f3 Group commit, gRPC roles, /metrics, beastbench and security policy
Fix Windows CI after #2: metrics test, single-writer commit path
…erformance-3d83f3 Group commit, gRPC roles, /metrics, beastbench and security policy
Fix Windows CI after #2: metrics test, single-writer commit path
What changed
Security, usability and performance upgrades from a review of the engine (spec:
docs/upgrade-spec.md).Performance
WAL.EnsureDurablebefore every dirty-page write (eviction,FlushPage,FlushAll). A failing barrier blocks the page write. Regression tests ininternal/storage/wal_barrier_test.go.cmd/beastbench. YCSB-style workloads A–F over gRPC with a latency histogram,--jsonoutput, a self-contained smoke mode, anddocs/benchmarking.mdon how to compare fairly with other databases.Security
viewer(Get/Scan),writer(+Put/Delete/BatchWrite),admin,replication(StreamWAL/Ack only). Unknown methods are denied for every role. Optional token expiry;POST /api/tokensacceptsexpires_in_secondsand rejects unknown roles with 400./metrics. Hand-written exposition, no new dependency. Never anonymous: admin session or a metrics/static bearer token.SECURITY.mdwith a private vulnerability reporting policy.Docs and housekeeping
GEMINI.md,CONTRIBUTING.md, newAGENTS.md/CLAUDE.md,demo.ps1, phase docs). Splitwal.go,buffer_pool.goand the beastbench runner along responsibility seams, with no behaviour change.Why
The per-write fsync (~4 ms) and the global engine lock were the performance ceilings, gRPC authentication ignored token roles (a viewer token could write), and there was no metrics endpoint or security policy.
Things a reviewer should know
replicationoradmintoken; tokens with a role outside the four now getPERMISSION_DENIED;POST /api/tokensreturns 400 for unknown roles; any gRPC method not inmethodRolesis denied.engine.goandbuffer_pool.goconflicted between group commit and metrics; both sides were additive and both are kept.go build,go vetandgo test ./... -racepass exceptTestEngineSystemWorkloadBenchmark(a fixed 1000 ops/s floor), which failed in two full parallel runs on a loaded laptop. Run alone it passes (about 1,875 ops/s here vs about 1,958 on the original code, so single-writer throughput is roughly 4% lower). I did not run the original code under the same full-suite load, so I can't say whether the merge makes that test more load-sensitive.StreamWALcan briefly read an appended-but-unsynced record; no encryption at rest yet.expires_in_seconds, a Studio expiry field, and a real competitor benchmark. The Studio redesign is a Figma design only;studio/is unchanged.SECURITY.mdworks as written.🤖 Generated with Claude Code