hudi-agent commented on code in PR #19317:
URL: https://github.com/apache/hudi/pull/19317#discussion_r3686865867
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java:
##########
@@ -253,6 +298,442 @@ protected HoodieMetadataPayload(String key,
HoodieSecondaryIndexInfo secondaryIn
this(key, MetadataPartitionType.SECONDARY_INDEX.getRecordType(), null,
null, null, null, secondaryIndexMetadata,
secondaryIndexMetadata.getIsDeleted());
}
+ protected HoodieMetadataPayload(String key, Object vectorIndexInfo) {
+ this.key = key;
+ this.type = MetadataPartitionType.VECTOR_INDEX.getRecordType();
+ this.vectorIndexMetadata = vectorIndexInfo;
+ this.isDeletedRecord = vectorIndexInfo instanceof
HoodieVectorIndexTombstone;
+ }
+
+ /**
+ * Create the generation-one centroid record for the given index partition.
+ */
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexCentroidsRecord(
+ ByteBuffer centroidBytes, String partitionPath) {
+ HoodieVectorIndexCentroids centroids = new HoodieVectorIndexCentroids(
+ 0L,
+ ByteBuffer.allocate(0),
+ centroidBytes,
+ ByteBuffer.allocate(0));
+ HoodieMetadataPayload payload = new
HoodieMetadataPayload(VectorIndexMetadataKey.centroids(1, 0L, 0), centroids);
+ HoodieKey key = new HoodieKey(VectorIndexMetadataKey.centroids(1, 0L, 0),
partitionPath);
+ return new HoodieAvroRecord<>(key, payload);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexCentroidsRecord(
+ int generation,
+ long centroidEpoch,
+ int chunk,
+ ByteBuffer clusterIds,
+ ByteBuffer centroidBytes,
+ ByteBuffer clusterRadii,
+ String partitionPath) {
+ String recordKey = VectorIndexMetadataKey.centroids(generation,
centroidEpoch, chunk);
+ HoodieVectorIndexCentroids centroids = new HoodieVectorIndexCentroids(
+ centroidEpoch, clusterIds, centroidBytes, clusterRadii);
+ return new HoodieAvroRecord<>(
+ new HoodieKey(recordKey, partitionPath),
+ new HoodieMetadataPayload(recordKey, centroids));
+ }
+
+ /**
+ * Create the generation-one quantizer metadata record for the given index
partition.
+ */
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexQuantizerMetadataRecord(
+ String quantizerType,
+ int quantizedCodeBytes,
+ long randomSeed,
+ boolean assumeNormalized,
+ String partitionPath) {
+ return createVectorIndexQuantizerMetadataRecord(
+ quantizerType,
+ quantizedCodeBytes,
+ 1,
+ randomSeed,
+ assumeNormalized,
+ partitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexQuantizerMetadataRecord(
+ String quantizerType,
+ int quantizedCodeBytes,
+ int rabitqBits,
+ long randomSeed,
+ boolean assumeNormalized,
+ String partitionPath) {
+ return createVectorIndexQuantizerMetadataRecord(1, 0, quantizerType,
randomSeed, null, partitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexQuantizerMetadataRecord(
+ int generation,
+ int chunk,
+ String quantizerType,
+ long randomSeed,
+ ByteBuffer rotationBytes,
+ String partitionPath) {
+ String recordKey = VectorIndexMetadataKey.quantizer(generation, chunk);
+ HoodieVectorIndexQuantizer quantizer = new
HoodieVectorIndexQuantizer(quantizerType, randomSeed, rotationBytes);
+ return new HoodieAvroRecord<>(
+ new HoodieKey(recordKey, partitionPath),
+ new HoodieMetadataPayload(recordKey, quantizer));
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexManifestRecord(
+ int generation,
+ String quantizerType,
+ int quantizedCodeBytes,
+ long randomSeed,
+ boolean assumeNormalized,
+ long lastUpdatedTs,
+ String metadataPartitionPath) {
+ return createVectorIndexManifestRecord(
+ generation,
+ quantizerType,
+ quantizedCodeBytes,
+ 1,
+ randomSeed,
+ assumeNormalized,
+ lastUpdatedTs,
+ metadataPartitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexManifestRecord(
+ int generation,
+ String quantizerType,
+ int quantizedCodeBytes,
+ int rabitqBits,
+ long randomSeed,
+ boolean assumeNormalized,
+ long lastUpdatedTs,
+ String metadataPartitionPath) {
+ return createVectorIndexManifestRecord(
+ generation,
+ String.valueOf(generation),
+ "ACTIVE",
+ 0,
+ 0,
+ quantizedCodeBytes,
+ rabitqBits,
+ Math.max(0, rabitqBits - 1),
+ 0,
+ 0,
+ "COSINE",
+ assumeNormalized,
+ false,
+ "",
+ 524288,
+ 0,
+ 0,
+ 0,
+ 0L,
+ lastUpdatedTs,
+ metadataPartitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexManifestRecord(
+ int generation,
+ String generationOrdinalText,
+ String state,
+ int dim,
+ int dimPadded,
+ int codeRowBytes,
+ int bitsTotal,
+ int numExPlanes,
+ int numClusters,
+ int shardCount,
+ String metric,
+ boolean assumeNormalized,
+ boolean residualEncoding,
+ String vectorColumn,
+ int targetBlockBytes,
+ int vectorsPerBlock,
+ int splitLimit,
+ int mergeFloor,
+ long centroidEpoch,
+ long createdTs,
+ String metadataPartitionPath) {
+ String recordKey = VectorIndexMetadataKey.manifest(generation);
+ HoodieVectorIndexManifest manifest = new HoodieVectorIndexManifest(
+ 1,
+ generationOrdinalText,
+ state,
+ dim,
+ dimPadded,
+ codeRowBytes,
+ bitsTotal,
+ numExPlanes,
+ numClusters,
+ shardCount,
+ metric,
+ assumeNormalized,
+ residualEncoding,
+ vectorColumn == null ? "" : vectorColumn,
+ targetBlockBytes,
+ vectorsPerBlock,
+ splitLimit,
+ mergeFloor,
+ centroidEpoch,
+ createdTs);
+ return new HoodieAvroRecord<>(
+ new HoodieKey(recordKey, metadataPartitionPath),
+ new HoodieMetadataPayload(recordKey, manifest));
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexGenerationManifestRecord(
+ int generation,
+ String quantizerType,
+ int quantizedCodeBytes,
+ long randomSeed,
+ boolean assumeNormalized,
+ long lastUpdatedTs,
+ String metadataPartitionPath) {
+ return createVectorIndexGenerationManifestRecord(
+ generation,
+ quantizerType,
+ quantizedCodeBytes,
+ 1,
+ randomSeed,
+ assumeNormalized,
+ lastUpdatedTs,
+ metadataPartitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexGenerationManifestRecord(
+ int generation,
+ String quantizerType,
+ int quantizedCodeBytes,
+ int rabitqBits,
+ long randomSeed,
+ boolean assumeNormalized,
+ long lastUpdatedTs,
+ String metadataPartitionPath) {
+ return createVectorIndexManifestRecord(
+ generation,
+ quantizerType,
+ quantizedCodeBytes,
+ rabitqBits,
+ randomSeed,
+ assumeNormalized,
+ lastUpdatedTs,
+ metadataPartitionPath);
+ }
+
+ public static HoodieRecord<HoodieMetadataPayload>
createVectorIndexClusterManifestRecord(
Review 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.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java:
##########
@@ -210,6 +254,7 @@ public class HoodieMetadataPayload implements
HoodieRecordPayload<HoodieMetadata
protected HoodieMetadataColumnStats columnStatMetadata = null;
protected HoodieRecordIndexInfo recordIndexMetadata;
protected HoodieSecondaryIndexInfo secondaryIndexMetadata;
+ protected Object vectorIndexMetadata;
Review 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.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -213,6 +215,71 @@ public class HoodieTableMetadataUtil {
HoodieRecord.HoodieMetadataField.PARTITION_PATH_METADATA_FIELD.getFieldName(),
HoodieRecord.HoodieMetadataField.COMMIT_TIME_METADATA_FIELD.getFieldName()));
+ public static String getVectorIndexGenerationManifestKey(int generationId) {
+ return VectorIndexMetadataKey.manifest(generationId);
+ }
+
+ public static boolean isVectorIndexGenerationManifestKey(String recordKey) {
+ return hasVectorIndexFamily(recordKey,
VectorIndexMetadataKey.FAMILY_MANIFEST);
+ }
+
+ public static boolean isVectorIndexQuantizerKey(String recordKey) {
+ return hasVectorIndexFamily(recordKey,
VectorIndexMetadataKey.FAMILY_QUANTIZER);
+ }
+
+ public static boolean isVectorIndexCentroidsKey(String recordKey) {
+ return hasVectorIndexFamily(recordKey,
VectorIndexMetadataKey.FAMILY_CENTROIDS);
+ }
+
+ public static String getVectorIndexClusterKey(int generationId, int
clusterId) {
+ return VectorIndexMetadataKey.clusterStats(generationId, clusterId);
+ }
+
+ public static boolean isVectorIndexClusterKey(String recordKey) {
+ return hasVectorIndexFamily(recordKey,
VectorIndexMetadataKey.FAMILY_CLUSTER_STATS);
+ }
+
+ public static String getVectorIndexPostingKey(int generationId, int
clusterId, int shardId, String recordKey) {
+ return VectorIndexMetadataKey.postingDelta(generationId, clusterId,
shardId, recordKey);
+ }
+
+ public static String getVectorIndexPostingPrefix(int generationId, int
clusterId) {
+ return VectorIndexMetadataKey.postingPrefix(generationId, clusterId,
0).substring(0, 9);
Review 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.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/index/vector/PostingBlockBuilder.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.index.vector;
+
+import org.apache.hudi.avro.model.HoodieVectorIndexPostingBlock;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hudi.common.util.ValidationUtils.checkArgument;
+
+/**
+ * Streaming-friendly builder for one immutable vector posting block.
+ */
+public final class PostingBlockBuilder {
+
+ public static final int BLOCK_FORMAT_VERSION = 1;
+ public static final int SCALAR_FACTOR_COUNT = 6;
+ public static final int SCALAR_FACTOR_COUNT_WITH_VECTOR_NORM = 7;
+ public static final int ROW_LOCATOR_BYTES = 8;
+
+ private final int codeRowBytes;
+ private final int numExPlanes;
+ private final boolean includeVectorNorm;
+ private final List<Row> rows = new ArrayList<>();
+ private final Map<String, Integer> fileGroupDict = new LinkedHashMap<>();
+ private final Map<String, Integer> instantTimeDict = new LinkedHashMap<>();
+ private final List<String> partitionDict = new ArrayList<>();
+
+ public PostingBlockBuilder(int codeRowBytes, int numExPlanes) {
+ this(codeRowBytes, numExPlanes, false);
+ }
+
+ public PostingBlockBuilder(int codeRowBytes, int numExPlanes, boolean
includeVectorNorm) {
+ checkArgument(codeRowBytes > 0 && codeRowBytes % Long.BYTES == 0,
+ "codeRowBytes must be positive and long-aligned: " + codeRowBytes);
+ checkArgument(numExPlanes >= 0, "numExPlanes must be non-negative: " +
numExPlanes);
+ this.codeRowBytes = codeRowBytes;
+ this.numExPlanes = numExPlanes;
+ this.includeVectorNorm = includeVectorNorm;
+ }
+
+ public PostingBlockBuilder addRow(String recordKey,
+ byte[] signPlane,
+ byte[] exPlanes,
+ float fAdd1,
+ float fRescale1,
+ float err1,
+ float fAddEx,
+ float fRescaleEx,
+ float residualNorm,
+ String fileGroupId,
+ String instantTime,
+ String partitionPath,
+ long rowPosition) {
+ return addRow(
+ recordKey,
+ signPlane,
+ exPlanes,
+ fAdd1,
+ fRescale1,
+ err1,
+ fAddEx,
+ fRescaleEx,
+ residualNorm,
+ null,
+ fileGroupId,
+ instantTime,
+ partitionPath,
+ rowPosition);
+ }
+
+ public PostingBlockBuilder addRow(String recordKey,
+ byte[] signPlane,
+ byte[] exPlanes,
+ float fAdd1,
+ float fRescale1,
+ float err1,
+ float fAddEx,
+ float fRescaleEx,
+ float residualNorm,
+ Float vectorNorm,
+ String fileGroupId,
+ String instantTime,
+ String partitionPath,
+ long rowPosition) {
+ checkArgument(recordKey != null, "recordKey must not be null");
+ checkArgument(signPlane != null && signPlane.length == codeRowBytes,
+ "signPlane must be exactly codeRowBytes");
+ checkArgument(exPlanes != null && exPlanes.length == numExPlanes *
codeRowBytes,
+ "exPlanes must be numExPlanes * codeRowBytes");
+ checkArgument(rowPosition >= 0 && rowPosition <= 0xFFFFFFFFL,
+ "rowPosition must fit in unsigned int: " + rowPosition);
+ checkArgument(includeVectorNorm == (vectorNorm != null),
+ "vectorNorm presence must match block scalar layout");
+
+ int fileGroupIdx = fileGroupDictionaryIndex(fileGroupId, partitionPath);
+ int instantIdx = dictionaryIndex(instantTimeDict, instantTime);
+ rows.add(new Row(
+ recordKey,
+ signPlane.clone(),
+ exPlanes.clone(),
+ fAdd1,
+ fRescale1,
+ err1,
+ fAddEx,
+ fRescaleEx,
+ residualNorm,
+ vectorNorm == null ? 0.0f : vectorNorm,
+ fileGroupIdx,
+ instantIdx,
+ rowPosition));
+ return this;
+ }
+
+ public HoodieVectorIndexPostingBlock build() {
Review 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.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
