CNDB-17471: Add a test for index disk bytes increase between EC and FA index version - #2504
CNDB-17471: Add a test for index disk bytes increase between EC and FA index version#2504ekaterinadimitrova2 wants to merge 1 commit into
Conversation
Checklist before you submit for review
|
❌ Build ds-cassandra-pr-gate/PR-2504 rejected by Butler4 regressions found Found 4 new test failures
Found 3 known test failures |
pkolaczk
left a comment
There was a problem hiding this comment.
Looking at the code, I'm not sure if this is meant to be a test for our understanding of the formulas or a test for making sure the size increase from FusedPQ is reasonable.
If it's the former, the formulas should be 1-byte exact. But we're making some inaccurate assumptions in a few places, e.g. assuming N for hierarchy level-1 is 1 which is likely not true (severely underestimated; I doubt the level 1 has only 1 node).
If it's the latter, I think it's enough to check the major contributing factor which is level-0 inline PQ vectors (= M * N * maxDegree). And then just assume that the rest shouldn't take more than additional 10% and call it a day ;)
Besides that; maybe worth parameterizing this test by the number of dimensions, per number of vectors and per maxDegree maybe?
| // The delta is purely the FusedPQ addition: M×maxDegree bytes per node. | ||
| // | ||
| // Beyond the per-node term, two small fixed costs are added by FA: | ||
| // - writeSparseLevels (version==6): N_level1 × (writeInt(ordinal) + writeSourceFeature(M bytes)) | ||
| // = N_level1 × (4+M) bytes. With hierarchy N_level1 = graph.size(1) (O(log N) nodes). | ||
| // - File header (Header.size(), written twice — start + footer): | ||
| // FA adds one extra featureId slot (4 bytes) + FusedPQ.headerSize() = pq.compressorSize() | ||
| // = 4*(5+M) + 4*256*D bytes per write → delta per write = 4*(6+M) + 4*256*D | ||
| // | ||
| // Lower bound (per-node term only): N × M × maxDegree | ||
| // Exact formula (all contributors): N×M×maxDegree + N_level1×(4+M) + 2×(4×(6+M) + 4×256×D) | ||
| // N_level1 and the internal jvector overhead are absorbed by the 1% tolerance. | ||
| long minTermsDataDelta = (long) N * (long) M * maxDegree; | ||
| long headerDeltaPerWrite = 4L * (6 + M) + 4L * 256 * DIMENSIONS; | ||
| // N_level1 is not directly accessible without a getter on CassandraDiskAnn; use 1 as a | ||
| // conservative lower-bound placeholder. The 1% tolerance absorbs the actual level-1 cost. | ||
| long computedTermsDataDelta = (long) N * (long) M * maxDegree | ||
| + (4L + M) | ||
| + 2L * headerDeltaPerWrite; |
There was a problem hiding this comment.
This feels a bit overcomplicated and slightly inconsistent. We go a long way towards computing the size of the headerDelta accurately, but then we ignore the log(n) level1 hierarchy contribution because we can't access all the params, and we just admit this is negligible because it will be smaller than 1%. In that case IMHO the only part we really care are the inline PQ vectors, that is N * M * maxDegree. All the rest looks like a noise. Headers contribute likely less than level 1 in the hierarchy.
I think the test should be simplified to just check that the total size increase is about N * M * maxDegree (with some tiny error; I think even 5% is ok here) and that's it.
Generally getting ~3.5 size increase regardless of the data size.
Kind of both. I do not think the approach with just checking whether it is 5% more or less is enough. Vector dimensions have a big importance as we see here. 3.5 - depends. The main goal for the first pass was to confirm I am not doing something totally off as the numbers went up eye watering... |
|
In ideal world, we should also see what is the difference from CA+ versions. |
The issue is CNDB-17471, I used 16942 by mistake for branch and commit. I will correct it when I am addressing review comments pre-final merge.
What is the issue
...
We need to keep track on how index disk size grows with index versions. It is also known that FA, due to FusedPQ is increasing the disk size significantly. The test here helps us approximate a formula for that and helps us catch later if indexes become even bigger.
What does this PR fix and why was it fixed
...
Adds
VectorFormatDiskUsageTest, a regression test that measures and validates the on-disk size growth of a SAI vector index when upgrading from formatECtoFA. (we should also add for previous versions but EC to FA was the immediate need)Why FA grows larger: FA introduces FusedPQ — pre-computed
PQdistance scores embedded inline in the graph file (TERMS_DATA), onePQcode per edge slot of every node. This makes approximate nearest-neighbour search faster by eliminating a separate file seek, but increasesTERMS_DATAbyN × m × Mbytes, where:N = number of indexed vectorsm = pq.compressedVectorSize()— bytes perPQ-encoded vector (determined by dimension; forD=256with default settings:100)M = maxDegree = 2 × maximumNodeConnections(default: 32)The full EC → FA storage delta per index is:
Δ ≈ N × m × M(dominant term —TERMS_DATA)+ 8 bytes(META: totalTermCountfield added in versionED)+ fixed header/sparse overhead (~0.5 MB, negligible)All other components (
PQ, POSTING_LISTS, COLUMN_COMPLETION_MARKER) are unchanged.Worked examples with default settings (
M=32):Note: enabling NVQ offsets some of this growth by replacing full-precision InlineVectors (D × 4 bytes/node) with quantized vectors (
NUM_SUB_VECTORS × (7 + D/NUM_SUB_VECTORS)bytes/node, default ~270 bytes forD=256), potentially recovering approximately 24% of the FusedPQ overhead forD=256- the numbers were provided by Bob and not verified yet.The current formula (enabling FusedPQ without NVQ) was validated against data (22.6M vectors, D=256): predicted 72.3 GB delta, observed 70 GB (3.2% error), confirming the model holds at scale.
The test writes the same
D=256vectors under bothECandFA, compacts to a single segment, and asserts:TERMS_DATAgrows byN × m × M (±1%)PQcomponent is identical in both versions after compactionMETAgrows by exactly 8 bytes (onetotalTermCountlong added in versionED)POSTING_LISTSandCOLUMN_COMPLETION_MARKERare unchangedConservation:
total delta = TERMS_DATA delta + 8 bytesFAis built withenable_hierarchy=trueto exercise the multi-level graph path.