feat(metadata): add MDT row families and payload schema for vector indexes - #19317
feat(metadata): add MDT row families and payload schema for vector indexes#19317chrevanthreddy wants to merge 4 commits into
Conversation
5e737cf to
7ef6a88
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19317 +/- ##
============================================
+ Coverage 70.49% 72.03% +1.54%
+ Complexity 32775 32614 -161
============================================
Files 2700 2567 -133
Lines 152896 148793 -4103
Branches 19009 18711 -298
============================================
- Hits 107778 107181 -597
+ Misses 36596 33128 -3468
+ Partials 8522 8484 -38
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This PR lays the MDT storage foundation for RFC-109 vector indexes — new Avro row families, a binary record-key codec (VectorIndexMetadataKey), posting-block encode/decode (PostingBlockBuilder/PostingBlockView), payload factory builders, and a new VECTOR_INDEX metadata partition type. It's additive scaffolding; the DDL/write/read paths land in follow-ups. I traced the key codec, the binary packing, the file-group routing, and the payload merge path (preCombine/combineMetadataPayloads) and they look internally consistent, and confirmed the mid-enum insertion is safe (no persisted ordinals). A couple of non-blocking items worth double-checking in the inline comments — round-trip test coverage for the posting-block packing, and some factory parameters that appear to be silently dropped. Please take a look at any inline comments, and this should be ready for a Hudi committer or PMC member to take it from here. A couple of naming and clarity suggestions below.
| return this; | ||
| } | ||
|
|
||
| public HoodieVectorIndexPostingBlock build() { |
There was a problem hiding this comment.
🤖 The posting-block packing here (struct-of-arrays scalar layout, little-endian offsets, dictionary indices, and the record-key offset table) is intricate and correctness-critical, but I don't see a PostingBlockBuilder -> PostingBlockView round-trip test in this PR. Could you add one that packs a few rows (including the includeVectorNorm variant and multi-plane exPlanes) and asserts every field decodes back? A silent offset error here would corrupt vector reads without failing loudly.
| metadataPartitionPath); | ||
| } | ||
|
|
||
| public static HoodieRecord<HoodieMetadataPayload> createVectorIndexClusterManifestRecord( |
There was a problem hiding this comment.
🤖 createVectorIndexClusterManifestRecord takes shardCount and fileGroupIds but neither appears to be written into the HoodieVectorIndexClusterStats record — the constructor gets (vectorCount, 0L, 0L, 0L, null, lastUpdatedTs) with the list slot passed as null. Is that intentional for this slice, or should those be persisted? A method named ...ClusterManifestRecord that silently drops the file-group membership it's handed seems like it could bite the write-path follow-up. Similarly the convenience createVectorIndexQuantizerMetadataRecord/createVectorIndexManifestRecord overloads drop randomSeed/assumeNormalized/quantizedCodeBytes — worth a comment noting those live on other records so a future caller doesn't assume they're persisted here.
| protected HoodieMetadataColumnStats columnStatMetadata = null; | ||
| protected HoodieRecordIndexInfo recordIndexMetadata; | ||
| protected HoodieSecondaryIndexInfo secondaryIndexMetadata; | ||
| protected Object vectorIndexMetadata; |
There was a problem hiding this comment.
🤖 nit: could you introduce a marker interface (e.g. HoodieVectorIndexEntry) implemented by the valid Avro types (HoodieVectorIndexPostingDelta, HoodieVectorIndexPostingBlock, HoodieVectorIndexCentroids, etc.) and type this field as that interface instead of Object? Right now there's no way to know which types are legal here without reading every factory method.
| } | ||
|
|
||
| public static String getVectorIndexPostingPrefix(int generationId, int clusterId) { | ||
| return VectorIndexMetadataKey.postingPrefix(generationId, clusterId, 0).substring(0, 9); |
There was a problem hiding this comment.
🤖 nit: the 9 here (and the identical .substring(0, 9) in VectorPostingPrefixRawKey) is a magic number derived from family(1) + generation(4) + clusterId(4). It might be worth surfacing it as a named constant in VectorIndexMetadataKey, e.g. CLUSTER_PREFIX_BYTES = 9, so both call sites stay in sync and the derivation is self-documenting.
7ef6a88 to
39bb0ed
Compare
|
Rebased this PR onto current Key corrections:
Validation on Java 17:
The earlier UTF-8 comparator compilation failure was caused by a stale locally installed shaded |
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR lays down the MDT storage model and payload schema for RFC-109 vector indexes — the Avro row families, the family-tagged binary key codec, the packed posting-block encode/decode, and the VECTOR_INDEX partition wiring. The binary key codec and posting-block serde are carefully constructed and have solid round-trip test coverage. One tombstone-serialization edge case is worth double-checking in the inline comment. Please take a look at any inline comments, and this should be ready for a Hudi committer or PMC member to take it from here. A few naming and readability suggestions below, most notably an overload that silently discards three of its own parameters.
| this.key = key; | ||
| this.type = MetadataPartitionType.VECTOR_INDEX.getRecordType(); | ||
| this.vectorIndexMetadata = vectorIndexInfo; | ||
| this.isDeletedRecord = vectorIndexInfo instanceof HoodieVectorIndexTombstone; |
There was a problem hiding this comment.
🤖 Setting isDeletedRecord = true for any HoodieVectorIndexTombstone means getInsertValue() returns Option.empty() for tombstones, so the tombstone payload built in createVectorIndexPostingDeleteRecord (with its deleteInstant/deleteReason) is never serialized — deletes degrade to plain delete markers and that provenance is dropped. Is that intended, given the schema models HoodieVectorIndexTombstone as a distinct union member with those fields? If tombstones are meant to be persisted and read back, this would need isDeletedRecord = false plus tombstone-aware handling in constructMetadataPayload/combineMetadataPayloads.
| String quantizerType, | ||
| int quantizedCodeBytes, | ||
| int rabitqBits, | ||
| long randomSeed, |
There was a problem hiding this comment.
🤖 nit: could you either pass quantizedCodeBytes, rabitqBits, and assumeNormalized through to the delegate, or remove them from this overload's signature? Right now the overload accepts those params and then silently drops all three, which will confuse any caller who passes non-default values.
| protected HoodieMetadataColumnStats columnStatMetadata = null; | ||
| protected HoodieRecordIndexInfo recordIndexMetadata; | ||
| protected HoodieSecondaryIndexInfo secondaryIndexMetadata; | ||
| protected Object vectorIndexMetadata; |
There was a problem hiding this comment.
🤖 nit: the other metadata fields (HoodieMetadataBloomFilter, HoodieSecondaryIndexInfo, etc.) are all strongly typed — have you considered a common marker interface for the vector payloads so vectorIndexMetadata doesn't have to be Object? That would save callers from unchecked casts.
| @Override | ||
| public String encode() { | ||
| return shardId == null | ||
| ? VectorIndexMetadataKey.postingPrefix(generationId, clusterId, 0).substring(0, 9) |
There was a problem hiding this comment.
🤖 nit: the bare 9 here (and at HoodieTableMetadataUtil.getVectorIndexPostingPrefix(int, int)) is easy to miss if the binary key layout ever changes — could you expose a named constant like POSTING_CLUSTER_PREFIX_BYTES from VectorIndexMetadataKey, or add a postingClusterPrefix(generation, clusterId) helper to keep the magic in one place?
|
Follow-up schema finalization is now pushed as
Java 17 validation after this change:
|
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the updates! This revision expands createVectorIndexManifestRecord with the new generation-scoped fields (fileGroupCount, blockFormatVersion, factorVersion, kappa/gMin/eps1Max/epsNRel, centroidChunkCount, centroidChecksum) and drops the earlier convenience overloads. I traced the positional HoodieVectorIndexManifest constructor call field-by-field against the Avro schema order and all 30 fields line up exactly, so there's no silent field misalignment — that was the main risk here. The removed overloads have no remaining callers and the test was updated consistently. One prior reviewer comment (cluster manifest not writing shardCount/fileGroupIds) now appears addressed. A few earlier comments remain open but weren't part of these changes and are already on the PR: the tombstone serialization question (isDeletedRecord=true means tombstones never persist their deleteReason/deleteInstant), the Object vectorIndexMetadata typing/marker-interface suggestion, the quantizer overload that still accepts quantizedCodeBytes/rabitqBits/assumeNormalized (fields the quantizer schema doesn't even model), and the magic 9 in the posting-prefix key logic. No new issues flagged from this pass. Please take a look at the still-open inline comments, and this should be ready for a Hudi committer or PMC member to take it from here.
| {"name": "indexVersion", "type": "int"}, | ||
| {"name": "generationId", "type": "string", "doc": "Creating Hudi instant + optional human tag. The gen ordinal lives in the key."}, | ||
| {"name": "state", "type": "string", "doc": "BUILDING | ACTIVE | RETIRED. Readers serve only the generation referenced by the singleton active manifest."}, | ||
| {"name": "dim", "type": "int"}, |
6 participants
Describe the issue this Pull Request addresses
Resolves #19097 (umbrella: #19094, RFC-109)
Second foundational slice of RFC-109 (Hudi Native Vector Index): the Metadata Table (MDT) storage model and payload schema for vector indexes. Builds on the option surface in #19096. DDL command dispatch, bootstrap/write execution, and the read path land in dependent follow-up PRs (#19098, #19099, #19100…).
Summary and Changelog
HoodieMetadata.avsc): add theVectorIndexMetadataunion with all typed row families —HoodieVectorIndexManifest,HoodieVectorIndexQuantizer,HoodieVectorIndexCentroids,HoodieVectorIndexPostingBlock,HoodieVectorIndexPostingDelta,HoodieVectorIndexClusterStats,HoodieVectorIndexTombstone. Encoding metadata (RaBitQ bits, seed, residual/extended codes, scalars) is versioned per generation for multibit-aware reads.HoodieMetadataPayload: newvectorIndexMetadatafield + offset-based serde, field/entry-type constants, dedicated constructor, and factory builders for centroid/quantizer/manifest/cluster/posting records.VectorIndexMetadataKey(family-tagged binary key codec) +VectorClusterRawKey,VectorGenerationManifestRawKey,VectorPostingPrefixRawKey(RawKeyimpls).HoodieTableMetadataUtil: binary key-family predicates/getters and cluster-aware file-group routing (mapVectorPostingKeyToFileGroupIndex).MetadataPartitionType.VECTOR_INDEX: new partition type (ordinal 8) wired for availability, payload construction, partition path, and file-group mapping.PostingBlockBuilder/PostingBlockView: packed posting-block encode/decode over the generatedHoodieVectorIndexPostingBlock.TestVectorIndexMetadataPayload(2) round-trip payload serde;TestVectorIndexMetadataKey(6) binary key encode/decode. ExistingHoodieMetadataRecordpositional callers updated for the new field.Impact
Additive to the MDT schema (new optional union field, default
null) and a new metadata partition type that is only populated via explicit vectorCREATE INDEX(not enabled by any metadata config flag). No behavioral change to existing partitions, writers, or readers.Risk Level
low
Schema change is an optional field defaulting to
null(backward/forward compatible); new partition type is inert unless a vector index is created.hudi-commoncompiles clean and vector serde + affected existing serde tests pass on current master.Documentation Update
Row families and encoding-metadata semantics are documented as Avro
docfields and Javadoc. User-facing RFC document tracked in #19309.Contributor's checklist