Speed up durable wire ingest path#134
Conversation
… GC interval jsonValue previously broke out of the loop on unterminated strings then accessed json[start..i+1] where i could equal json.len — one byte past the buffer. In ReleaseFast (no bounds checks) this is UB/segfault. Now returns early on closing quote and returns null on unterminated input. Also lowers MVCC GC_INTERVAL from 10,000 to 500 to prevent OOM during large bulk ingests — version chains now get cleaned up 20x more often. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
handleConn only had a read loop for bulk bodies > 64KB (big_buf path). For bodies <= 64KB it did a single read() and assumed TCP delivered everything — but TCP can fragment data across packets, so most of the NDJSON body was never read. This caused ~85% of bulk insert lines to be silently skipped (no key found → continue). Now small bulk requests also loop until Content-Length bytes are received, using the existing req buffer without extra allocation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eader
The old parser only handled C/c for the first character but used a
case-sensitive match for the rest ("ontent-length: "). curl and most
HTTP clients send "Content-Length" (capital L), which didn't match
"content-length" (lowercase l). This meant the bulk read loop never
triggered for bodies > 64KB — the server only processed the first
read() chunk and silently dropped the rest of the NDJSON payload.
Now matches each character of "Content-Length:" independently.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dropCollectionForTenant removed the collection from the in-memory hashmap but left the .pages file on disk. The next access to the same collection name re-opened the old file and all data reappeared, making DROP appear to do nothing. Now reconstructs the page file path and unlinks it after closing the collection, so data is actually gone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… GC interval Cap query limits to prevent client-driven OOM: scan max 1000, search max 500, context max 100. Add buffer overflow guards in scan/search response serialization — stop writing docs when the 64KB body buffer is nearly full instead of silently truncating JSON. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Cap query limits: scan max 1000, search max 500, context max 100 - Add buffer overflow guards in scan/search/context response loops — stop serializing when the 64KB body buffer is nearly full instead of silently truncating JSON - Bulk insert now checks tenant storage quota before processing (was bypassing ensureTenantStorageAvailable entirely) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Trigram index: cap posting lists at 10K entries per trigram. Trigrams
appearing in more docs than this are too common to be discriminative
for search — skip them to prevent unbounded memory growth on large
collections (13K+ files could otherwise consume hundreds of MB).
Word index: cap hits per word at 5K. Common words ("the", "var", "if")
accumulate thousands of hits across a codebase — beyond this limit they
waste memory for negligible search value.
WAL: truncate the file after checkpoint. All data is already durable
in page files at checkpoint time, so the WAL can be safely cleared to
reclaim disk space. Prevents unbounded WAL growth on long-running
instances.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Throughput and stability under high concurrent load: - WAL: add 8MB write buffer backpressure — writers yield when buffer is full instead of growing unboundedly toward OOM - Thread stack: 256KB explicit stack size (was OS default ~2MB). 512 threads × 2MB = 1GB just for stacks; now 512 × 256KB = 128MB - Connection rejection: return HTTP 503 with JSON body instead of silent TCP RST when at MAX_CONNECTIONS - Stripe locks: 1024 → 4096 to reduce contention under 100+ concurrent writers (birthday paradox collision rate drops 4x) - Index queue: 32K → 128K capacity to absorb write bursts before falling back to synchronous indexing - Second index worker thread spawned to double indexing throughput Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Clients can upgrade any HTTP connection to WebSocket by sending a
standard Upgrade: websocket request. After the 101 handshake, each
WS text message is dispatched as a request using the format:
METHOD /path\n{body}
e.g. "POST /db/mycol\n{\"key\":\"k\",\"value\":{}}"
The response body (without HTTP headers) is sent back as a WS text
frame. This enables:
- Streaming bulk inserts over one connection (bypasses Cloudflare
Workers 1000 subrequest limit)
- Lower latency (no HTTP overhead per request)
- Persistent connections for real-time operations
Implementation: SHA1+Base64 handshake, RFC 6455 frame parsing with
masking, ping/pong keepalive, 16MB max message (heap-allocated).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… GC interval Four correctness and safety fixes: CDC: workerMain captured subscriptions slice pointer then released the lock — registerWebhook could reallocate the ArrayList, dangling the pointer. Now iterates by index, re-reading under lock each iteration. WAL: backpressure yield-loop gave up after 100 retries and let writes proceed anyway, defeating the purpose. Now uses cond.wait() so writers truly block until the flusher drains below threshold. doc.zig: decode() now rejects val_len > 64MB as corrupt before using it in size arithmetic, preventing OOB slice creation from corrupted page data. Moved key_len check before the total size calculation. branch.zig: write() freed old entry before allocating new copies — if the allocation failed, the hashmap entry had dangling pointers. Now allocates first, frees second, with proper errdefer on both copies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…threads WordIndex had no synchronization. With two background index worker threads both calling words.indexFile() concurrently, the unsynchronized StringHashMap operations (getOrPut, append, put) corrupted internal state during resize — causing a panic or segfault in ReleaseSafe. This was the root cause of the vitess ingest crash (msg #41): the second index worker (spawned in commit 5190777) raced with the first on the shared WordIndex, corrupting the hashmap. Added mu: Mutex to WordIndex, acquired in indexFile() and search(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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: a0bc4bf27a
ℹ️ 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".
| @memcpy(zbuf[0..path.len], path); | ||
| zbuf[path.len] = 0; |
There was a problem hiding this comment.
In runUnix, the code copies path into a fixed [256]u8 buffer before any length validation. If path.len >= 256, @memcpy(zbuf[0..path.len], path) and zbuf[path.len] = 0 write out of bounds; and this happens before the later PathTooLong guard for the sockaddr path length. This can crash in safety builds or corrupt stack memory in release builds when a long Unix socket path is passed.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .nanoapi = .{ | ||
| .path = "../nanoapi", | ||
| }, |
There was a problem hiding this comment.
🔴 Local path dependency ../nanoapi breaks CI builds
The build.zig.zon file declares a dependency on nanoapi using a relative local path (.path = "../nanoapi"). The CI workflows (.github/workflows/ci.yml and benchmark.yml) only check out the turbodb repository via actions/checkout@v4 without cloning the nanoapi dependency. Since src/server.zig:23 unconditionally imports nanoapi and build.zig:7-11 unconditionally resolves the dependency, zig build will fail in CI with a missing dependency error. Confirmed that ../nanoapi does not exist in the checked-out environment.
Prompt for agents
The nanoapi dependency uses a local path (.path = "../nanoapi") which only works when the developer has it cloned as a sibling directory. CI workflows only checkout this repo so the build will fail. Options to fix: (1) Vendor nanoapi into this repo (e.g. as a git submodule or directly), (2) Use a URL-based dependency (.url = "https://...") pointing to the nanoapi repository with a hash, (3) Add a CI step to clone nanoapi as a sibling before building. The most robust fix for reproducibility is option (2) using a .url + .hash dependency.
Was this helpful? React with 👍 or 👎 to provide feedback.
| var walker = dir.iterate(); | ||
| // iterate needs no deinit |
There was a problem hiding this comment.
🔴 Recursive directory walk replaced with flat iterate, breaking subdirectory indexing
The cmdIndex function previously used dir.walk(alloc) which recursively traverses all subdirectories to index source files. It was replaced with dir.iterate() which only lists immediate children of the given directory. The compat.IterableDir.Iterator only wraps readdir() — no recursive descent. This means tdb index <dir> will silently skip all files in subdirectories, which is a severe functional regression since codebases always have nested directory structures. The same issue affects src/profile_index.zig:90 and src/scale_bench.zig:63.
Prompt for agents
The compat layer's IterableDir only provides a flat iterate() based on readdir(). The old code used std.fs.Dir.walk() which recursively descends into subdirectories. A recursive walk is essential for the tdb index, profile_index, and scale_bench tools since codebases have nested directories. To fix this, implement a recursive walk in the compat layer (compat.zig). This could be done by maintaining a stack of DIR* handles and descending into subdirectories as they are encountered, or by implementing a Walker struct similar to std.fs.Dir.Walker. The fix needs to be applied in three files: src/tdb.zig (cmdIndex), src/profile_index.zig (main), and src/scale_bench.zig (main).
Was this helpful? React with 👍 or 👎 to provide feedback.
| if errors.value: | ||
| raise RuntimeError(f"FFI bulk update had {errors.value} row errors") | ||
|
|
||
| return timed_batch_loop(self.batches(samples), op, {"mode": "embedded_c_abi_bulk_update", "latency_unit": "amortized_row"}) |
There was a problem hiding this comment.
🟡 Missing batches method on TurboDBFFIBench causes AttributeError
TurboDBFFIBench.update_orders() (line 501) and TurboDBFFIBench.delete_orders() (line 518) call self.batches(samples), but the TurboDBFFIBench class (defined at line 308) does not define a batches method and does not inherit from any class. The TurboDBBench class (line 195) and TigerBeetleBench class (line 714) both define their own batches methods, but TurboDBFFIBench does not. This will raise AttributeError: 'TurboDBFFIBench' object has no attribute 'batches' at runtime when the update or delete benchmark workloads are executed.
Prompt for agents
The TurboDBFFIBench class at line 308 in bench/container_shape_bench.py is missing a batches() method that is called by update_orders (line 501) and delete_orders (line 518). Add the same batches method that TurboDBBench has at line 195:
def batches(self, values: list[Any]) -> list[list[Any]]:
batch_size = max(self.batch_size, 1)
return [values[i:i + batch_size] for i in range(0, len(values), batch_size)]
This should be added as a method on TurboDBFFIBench, or alternatively extracted into a shared mixin/base class since TurboDBBench and TigerBeetleBench both have the same implementation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn deleteTree(_: Dir, sub_path: []const u8) !void { | ||
| var cmd_buf: [4200]u8 = undefined; | ||
| const cmd = std.fmt.bufPrint(&cmd_buf, "rm -rf '{s}'\x00", .{sub_path}) catch return error.NameTooLong; | ||
| _ = system(@ptrCast(cmd.ptr)); | ||
| } |
There was a problem hiding this comment.
🟡 Shell injection in compat.Dir.deleteTree via unescaped single quotes in path
The deleteTree implementation constructs a shell command using single-quote escaping (rm -rf '{s}') but does not escape single quotes within the path string itself. If sub_path contains a single quote character, the shell command breaks out of quoting, enabling command injection. For example, a path containing '; malicious_cmd; echo ' would be executed as a separate shell command. While current callers use hardcoded paths, this is a general-purpose compatibility function that could be called with dynamic paths in the future.
Prompt for agents
The deleteTree function in src/compat.zig:329-333 uses system("rm -rf '...'") which is vulnerable to shell injection if the path contains single quotes. A safer implementation would recursively delete using opendir/readdir/unlink/rmdir POSIX calls directly (similar to how the std library implements it), or at minimum properly escape single quotes by replacing each ' with '\'' in the path before interpolation. Using nftw() from POSIX would be another safe approach that avoids shell entirely.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
/ingestProduction validation
Deployed on
65.109.55.143:11a669716077eb9e3a0d522f8c671460a3fc6ea9bb6ff884fce3be2e863de1fc51e5e37dc8a51ec99b7e1b165283a52e25a3ced19dc43cabb0d33eac55304364Stress results through public Cloudflare/Caddy/cloudflared path:
c128/r128/b4096: 524,288 records, 51.2k durable records/sec, 0 failed records, 0 wire batch failuresc256/r256/b4096: 1,048,576 records, 40.5k durable records/sec, 0 failed records, 0 wire batch failuresLocal verification
zig buildzig build test-wirezig build test-allzig build -Dtarget=x86_64-linux -Doptimize=ReleaseFastIntegration note
This branch is intentionally opened as an integration PR instead of direct-pushing to
main. It contains the deployed engine perf line and is currently broader than a minimal cherry-pick becausemaindoes not yet have the durable bulk insert / wire batch scaffolding that the deployed path depends on.