fix(vector): SQ8/TQ code-size mis-dispatch CPU error-storm (#73)#334
Merged
Conversation
… memory accounting SQ8's `QuantizationConfig::bits()` returns 8, which is OUTSIDE the TurboQuant 1..=4 range that the free `code_bytes_per_vector(padded, bits)` supports. On the `_ =>` arm that function returned 0 and logged `tracing::error!` on EVERY call. `CollectionMetadata::code_bytes_per_vector()` fell through to that free fn for SQ8, so any unconditional caller hit the 0-return + per-call error log. The hot memory-accounting paths (`store.rs` used_memory / eviction checks: `live_count * bytes_per_code_per_vector()`) call it repeatedly, turning a benign config into a CPU-pegging error storm and an under-count of SQ8 resident bytes (reported ~4 B/vec instead of dim+8). Observed 2026-07 on a lunaris-backed SQ8 store (one moon instance pegged a core; log flooded with the codebook error). Root cause is a size-dispatch gap, not io busy-poll. SQ8 stores `dim` u8 codes (TRUE, unpadded dimension) + an 8-byte (min,scale) trailer — as `MutableSegment::new` and `compaction.rs` already special-case inline. The collection-level helpers did not. Fix (comprehensive, host-tested): - `CollectionMetadata::code_bytes_per_vector()`: SQ8 -> `dimension` (true dim), matching the segment/compaction inline branches; TQ paths unchanged. - `CollectionMetadata::bytes_per_code_per_vector()`: SQ8 -> `dimension + SQ8_PARAMS_BYTES` (8-byte trailer), not `code_bytes()+4`. Corrects memory accounting (store.rs) and is consistent with the already-SQ8-guarded compaction slot sizing. - Free `code_bytes_per_vector`: log the unsupported-width error ONCE (AtomicBool) instead of per-call, so any future stray caller can never re-create the storm. Verified: FT.CREATE already rejects bogus quant strings (TQ8 etc.) at ft_create.rs:704 — the bits=8 origin was legitimate SQ8, so no create-path change needed. Compaction (compaction.rs:1143) already guards SQ8 explicitly, so it was never corrupted; the live bug was purely the unguarded memory-accounting calls. Tests: new `test_code_bytes_per_vector_sq8` (dim 768 -> 768/776; dim 384 -> 384/392). Pre-fix these returned 0/4. Existing TQ code-size tests unchanged. Refs #73. Branch held for PR until v0.7.0 is tagged (24h replication soak owns the CI VM). author: Tin Dang <tindang.ht97@gmail.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesSQ8 TurboQuant corrections
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: SQ8/TQ code-size mis-dispatch CPU error-storm
The bug
SQ8's
QuantizationConfig::bits()returns 8, which is outside TurboQuant'ssupported
1..=4range. The free helpercode_bytes_per_vector(padded, bits)fellthrough to its
_ =>arm, returned 0, and logged atracing::error!on everycall. Because that helper sits on the hot memory-accounting path (
store.rsviabytes_per_code_per_vector()), an SQ8 index produced:tracing::error!per accounting call), andThis was the real cause of the leaked-process CPU spike originally mis-attributed to
the I/O busy-poller.
The fix
Dispatch now computes the true SQ8 layout directly instead of routing through the
TurboQuant bit-width path:
code_bytes_per_vector()— SQ8 ⇒dim(true, unpadded)u8codes; TQ4 ⇒padded/4.bytes_per_code_per_vector()— SQ8 ⇒dim + SQ8_PARAMS_BYTES(8-byte affine(min,scale)trailer).code_bytes_per_vector_ =>arm now logs the unsupported-bit-width errorat most once via an
AtomicBoollatch (defensive; no longer on any live SQ8 path).Tests (red/green TDD)
New
test_code_bytes_per_vector_sq8asserts the real layout (dim 768 → 768/776;dim 384 → 384/392). Full vector suite green on the VM: 970 passed, 0 failed.
cargo fmt --checkclean;cargo clippy -- -D warningsclean on both default andruntime-tokio,jemallocfeature sets.Scope / risk
Memory-accounting + logging only — no change to SQ8 encode/decode/search/merge
correctness (compaction already guarded SQ8 at
compaction.rs:1143). AdditiveCHANGELOG entry under
[Unreleased].Closes #73.
Summary by CodeRabbit
Bug Fixes
Documentation