Skip to content

Bank striping across N devices: WASTE_BANK_SHARDS, record-granularity round-robin - #53

Merged
marcobambini merged 1 commit into
sqliteai:mainfrom
mfethe1:feature/lenny/bank-striping
Aug 28, 2026
Merged

Bank striping across N devices: WASTE_BANK_SHARDS, record-granularity round-robin#53
marcobambini merged 1 commit into
sqliteai:mainfrom
mfethe1:feature/lenny/bank-striping

Conversation

@mfethe1

@mfethe1 mfethe1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What

Optional multi-device expert-bank striping. Set WASTE_BANK_SHARDS=/mnt/a,/mnt/b and each layer's bank opens as N shard files (bank basename in each dir); expert e is read from shard e % N at offset (e / N) * rec_bytes. tools/split_banks.py produces and byte-verifies shard sets. Unset env = exact current behavior — unstriped is the N=1 case of the same code path.

Why

Per-token demand is top-k experts of a single layer — one file today, so one device serves every expert read of a token. Round-robin record placement spreads a token's reads across devices, so effective bandwidth scales toward min(N, readers) with the existing async reader pool (WASTE_IO_THREADS). On a 2-drive rig this targets ~1.8-2x; on 4 NVMe with more readers, more.

No container format change: shards are plain files named after each bank. Fail-closed: any missing or short shard refuses the load.

Evidence

  • tiny.waste split across 2 dirs: logits byte-identical to unstriped (sha256 de62689a…, argmax + max-logit equal) — correctness gate is identity, not tolerance
  • tools/split_banks.py --mode verify: every record byte-identical post-split
  • make check 48 passed / 0 failed (13 skips: no real container locally)
  • ASan+UBSan striped run: 0 findings, same-build striped==unstriped byte-exact

Not claimed

No device A/B numbers yet — needs a 2-drive rig with a real container. This PR is the mechanism; the bandwidth measurement follows on hardware.

@mfethe1
mfethe1 force-pushed the feature/lenny/bank-striping branch from 55948c0 to 09ada61 Compare August 27, 2026 08:07
WASTE_BANK_SHARDS=dirA,dirB,... opens each layer's bank as N shard files,
expert e on shard e%N at offset (e/N)*rec_bytes. Round-robin because the
per-token demand is k experts of ONE layer — layer-granularity placement
would leave every read of a token on one drive. Unstriped is the N=1 case
of the same code path; env unset = exact prior behavior.

- waste_bank: fd[16] + n_shards; all 6 deref sites migrated
- load: manifest-driven shard open, fail-closed on short shard sets
- bank_fetch: shard+offset resolution, byte-exact by construction
- tools/split_banks.py: split + byte-for-byte verify modes
- evidence: tiny.waste striped across 2 dirs, logits byte-identical
  (sha de62689a...), make check 48/0, ASan+UBSan striped run clean 0 findings
@mfethe1
mfethe1 force-pushed the feature/lenny/bank-striping branch from 09ada61 to bc710d5 Compare August 27, 2026 08:13
@mfethe1

mfethe1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Repointed the PR branch to a clean single-commit base on current main (878352b) — it was previously stacked on the unmerged #48 branch, which made the diff carry unrelated commits and left mergeable_state: dirty.

Verified on the rebased head before pushing: make check 44 passed / 0 failed / 13 skipped; split_banks.py split+verify roundtrip on a fresh make_test_container fixture (all records byte-identical across shards); logits byte-identical striped vs unstriped (--json sha256 match on both arms); ASan/UBSan clean in the local suite. CI is now 9/9 green on bc710d5 and the diff is exactly the three intended files (src/model.c, src/model.h, tools/split_banks.py). The striping behavior is unchanged from the original description — only the base and the missing SPDX header in the new tool are different.

marcobambini added a commit that referenced this pull request Aug 28, 2026
…d list

Two things found while resolving #53 against main, both in the direction
the PR already intended.

The comment said "every shard must exist and be exactly the size its share
of experts demands -- a short or long shard fails the load rather than
serving a wrong record." It did not. bank_open's rec_bytes parameter is
(void)-ed and has been since it was written, so nothing measured anything,
and a shard short by a whole record loaded happily: rc=0, and it generated
logits. Measured on a tiny.waste split across two dirs with one record cut
off the end of shard 0.

Those logits were *correct* -- the missing expert simply was not routed to
in an 8-token prompt -- which is the actual hazard and worse than a wrong
answer. The failure is latent and prompt-dependent: it waits for the router
to pick a high-numbered expert, which on K3 can be thousands of tokens in,
or never during a test and always in production.

So the check now exists: one WASTE_ALIGN read at the last record's offset
per shard, which is 4 KiB-aligned like every record and therefore legal
under O_DIRECT. 4 KiB per shard per layer, 92 reads on K3. The same probe
runs for the unstriped N=1 case, which is the point of that path being the
same code. A shard truncated *inside* its last record still fails at that
record's read, and a wrong N is still caught by the expert id in the record
header -- neither could ever be served as another expert's weights, and the
comment now says which failure happens where.

Second, WASTE_BANK_SHARDS was copied into a 1024-byte buffer with snprintf
and the truncation ignored. A truncated list silently drops shards, and a
shard count that disagrees with the split that produced them is a different
layout rather than a smaller one. It is refused.

Verified: striped and unstriped logits byte-identical on a two-way split,
a shard short by one record refused at load where it previously loaded and
ran, synthetic suite 61 passed / 0 failed / 9 skipped.

Co-Authored-By: Claude Opus 5 <[email protected]>
marcobambini added a commit that referenced this pull request Aug 28, 2026
Bank striping across N devices: WASTE_BANK_SHARDS, record-granularity
round-robin. Resolved against main -- the GLM work added two frees beside
the bank close that this branch turned into an fd array, and both sides
are kept -- then two follow-ups: the fail-closed claim in its own comment
was not true, and a truncated WASTE_BANK_SHARDS was ignored.

Verified striped == unstriped byte-identical on a two-way split, under
ASan/UBSan too; a shard short by one record now refused at load where it
previously loaded and generated; synthetic 61/0/9, K3 64/0/6.
@marcobambini
marcobambini merged commit 52dd848 into sqliteai:main Aug 28, 2026
9 checks passed
marcobambini added a commit that referenced this pull request Aug 28, 2026
Enforce shard size at open, companion to #53. Rebased onto the O_DIRECT
probe and complementary to it: size is the whole-file claim and needs no
read, so it also covers containers whose record size is not a whole number
of blocks, where the probe skips; the probe is the readability claim.

Landed with the justification corrected to what a negative control shows.
A misplaced record cannot be served as another expert's -- record_check
reads the expert id out of every record header and refuses. What the size
check buys is when: a long shard loaded and generated correct output
before it, and a short one waited for the router to reach the missing
expert.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants