nsivabalan commented on code in PR #9058:
URL: https://github.com/apache/hudi/pull/9058#discussion_r1246949788
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java:
##########
@@ -283,6 +285,8 @@ public HoodieMetadataPayload(Option<GenericRecord>
recordOpt) {
Integer.parseInt(recordIndexRecord.get(RECORD_INDEX_FIELD_FILE_INDEX).toString()),
Long.parseLong(recordIndexRecord.get(RECORD_INDEX_FIELD_INSTANT_TIME).toString()));
}
+ } else {
+ this.isDeletedRecord = true;
Review Comment:
hey folks. here is the context. I feel we should go this route. may be there
are opportunities to optimize col stats and bloom filter records as well.
Generally, for any payload, we should have a key and a top level field
preferrably to denote isDeleted.
So, if entire records needs to be deleted, we can rely on the top level
isDelete field. This is unavoidable since we write using
EmptyHoodieRecordPayload in some flows (delete), but read back using specific
payload class. So, every payload will have to support deserialize an
EmptyRecordPayload.
Now, lets go into more specifics.
RLI:
Commit1:
add key1 to RLI partition.
rolling back commit1:
delete key1 from RLI partition.
From a HoodieRecord standpoint, its as simple as adding a new entry and
deleting the same. Its simpler and our getInsertValue or
combineAndGetUpdateValue will be fast. If we push the isDeleted to
HoodieRecordIndexInfo, then we need to explicitly set the type and then parse
the HoodieRecordIndexInfo data and then deduce that its deleted.
Again, w/ EmptyRecordPayload, this is not even doable and we have to go with
this.
Why we did not have this issue before.
apparently, with FILES, the keys are partitions, and hence, except
delete_partition, no records from FILES will be deleted in its entirely.
W/ col stats, a delete, while writing to MDT partition, is yet another
upsert record with isDeleted within ColumnStats Metadata. So, our
getInsertValue or combineAndGetUpdate value will need to deserialize entire
record and then deduce that its deleted.
A right fix here also would be to do what we are doing w/ RLI in this patch.
i.e.
in commit1,
add col1_part1_file1 : value to MDT
in some X commit, when file1 is deleted:
just delete col1_part1_file1 from col stats partition in MDT, by using
EmptyRecordPayload.
So, Log record reading and compaction will be fast.
--
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]
