danny0405 commented on code in PR #869:
URL: https://github.com/apache/hudi-rs/pull/869#discussion_r4420103469
##########
crates/core/src/index/record.rs:
##########
@@ -76,40 +85,112 @@ struct LoadedLocation {
instant_time_millis: i64,
}
-async fn load_record_index_map(
+/// Locations of `record_keys`, read from only the index shards and HFile
+/// blocks that can hold them.
+///
+/// Keys are hashed to their `record_index` file group the way the writer
+/// placed them, so a lookup opens the touched shards rather than the whole
+/// partition, and inside a shard the HFile block index narrows the read to the
+/// blocks the keys fall in. Values are Avro-decoded only for the requested
keys.
+async fn lookup_record_index(
table: &Table,
storage: Arc<Storage>,
+ record_keys: &[&str],
) -> Result<HashMap<String, LoadedLocation>> {
let partition = MetadataPartitionType::RecordIndex.partition_name();
// Slice-resolved: only each file group's LIVE slice (multi-base aware
- // after a Spark MDT compaction), logs in version order.
- let mut base_paths = Vec::new();
- let mut log_paths = Vec::new();
- for (_, base, logs) in
- crate::write::metadata::mdt_partition_latest_slices(storage.as_ref(),
partition).await?
- {
- base_paths.extend(base);
- log_paths.extend(logs);
+ // after a Spark MDT compaction), logs in version order. One listing serves
+ // both the shard modulus and the routing, so the two cannot disagree.
+ let slices = mdt_partition_latest_slices(storage.as_ref(),
partition).await?;
+ let num_groups = shard_count_from_slices(
+ &slices,
+ RECORD_INDEX_FILE_ID_PREFIX,
+ DEFAULT_RLI_NUM_FILE_GROUPS,
+ );
+ let mut keys_by_shard: HashMap<String, Vec<String>> = HashMap::new();
+ for key in record_keys {
+ let shard = map_record_key_to_file_group_index(key, num_groups);
Review Comment:
Suggestion (non-blocking): consider documenting or strengthening the
missing-shard guard's limits. Correction to my original review: the P1
classification was too strong. The example below requires an already
incomplete/corrupt index; I have not established a supported table lifecycle
that produces it. Initialization creates all shards, and Java also derives its
routing modulus from the file-slice count.
The narrower observation still holds: with 10 original shards, existing key
`"d"` (hash 100) belongs to shard 0. Removing every file of shard 5 makes the
inferred count 9, so lookup routes `"d"` to existing shard 1 and the guard does
not fire. The old full scan could still find this key in surviving shard 0. I
confirmed the routing with the extracted Rust functions, not an end-to-end
write test.
If detecting missing-shard corruption is an intended guarantee of this PR,
consider validating gaps in the shard layout before routing and add a test
using an existing key in an intact shard. Otherwise, clarify that this guard
only detects a requested shard ID absent from the listing; it does not validate
overall index completeness. I would not block the PR on this without evidence
that the incomplete state can arise during supported operation.
--
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]
