vinishjail97 commented on code in PR #20188:
URL: https://github.com/apache/hudi/pull/20188#discussion_r4187462303
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/index/record/BaseRecordIndexer.java:
##########
@@ -392,19 +393,22 @@ public static <T> HoodieData<HoodieRecord>
convertMetadataToRecordIndexRecords(H
}
try {
- Map<String, List<HoodieWriteStat>> writeStatsByFileId =
allWriteStats.stream().collect(Collectors.groupingBy(HoodieWriteStat::getFileId));
- int parallelism = Math.max(Math.min(writeStatsByFileId.size(),
metadataConfig.getRecordIndexMaxParallelism()), 1);
+ // a file group is identified by its partition and file id, the file id
alone is only unique within a partition for files
+ // written outside Hudi
+ Map<HoodieFileGroupId, List<HoodieWriteStat>> writeStatsByFileGroupId =
allWriteStats.stream()
+ .collect(Collectors.groupingBy(writeStat -> new
HoodieFileGroupId(writeStat.getPartitionPath(), writeStat.getFileId())));
+ int parallelism = Math.max(Math.min(writeStatsByFileGroupId.size(),
metadataConfig.getRecordIndexMaxParallelism()), 1);
Review Comment:
Good catch, the base file branch already used each write stat's own
partition, so the old assertion proved nothing for the record index. Added
`TestMetadataUtilRLIandSIRecordGeneration#testRecordIndexRecordsForLogFilesWithTheSameFileIdInDifferentPartitions`
in a14aef0f6ad3. It deletes one record in each of two partitions of a MOR
table, gives both log file write stats one file id, and checks the partition of
each delete from `convertMetadataToRecordIndexRecords` with a partitioned
record index. Without the fix both deletes carry the first partition.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexRecordGenerationUtils.java:
##########
@@ -263,7 +267,7 @@ private static <T> HoodieData<HoodieRecord>
convertReplacedFileGroupsToSecondary
HoodieSchema tableSchema) {
List<Pair<String, String>> replacedFileGroups =
replaceCommitMetadata.getPartitionToReplaceFileIds().entrySet().stream()
.flatMap(partitionAndFileIds -> partitionAndFileIds.getValue().stream()
- .filter(fileId -> !writtenFileIds.contains(fileId))
+ .filter(fileId -> !writtenFileGroupIds.contains(new
HoodieFileGroupId(partitionAndFileIds.getKey(), fileId)))
Review Comment:
Added in a14aef0f6ad3: a second commit replaces `brazil` while
`americas/peru` writes a `part-00000.parquet`, then checks that `bob` and the
brazil keys are gone from both indexes. With only the filter reverted, the test
fails on the secondary index assertion.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/index/record/BaseRecordIndexer.java:
##########
Review Comment:
Done in a14aef0f6ad3.
--
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]
