yihua commented on code in PR #13305:
URL: https://github.com/apache/hudi/pull/13305#discussion_r2124546211
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/TestSparkRDDMetadataWriteClient.java:
##########
@@ -130,6 +130,8 @@ public void testWritesViaMetadataWriteClient() throws
Exception {
client.startCommitForMetadataTable(metadataMetaClient,
commitTimeOfInterest, DELTA_COMMIT_ACTION);
JavaRDD<WriteStatus> partialWriteStatusesRDD =
client.upsertPreppedRecords(jsc.parallelize(rliRecords), commitTimeOfInterest,
Option.of(nonFilesPartitionFileGroupIdList));
List<WriteStatus> partialWriteStatuses =
partialWriteStatusesRDD.collect();
+ // assert that isMetadata is rightly set
Review Comment:
```suggestion
// assert that `updatesMetadataTable` is set
```
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/TestSparkRDDMetadataWriteClient.java:
##########
@@ -140,6 +142,8 @@ public void testWritesViaMetadataWriteClient() throws
Exception {
// write to FILES partition
JavaRDD<WriteStatus> filePartitionWriteStatusesRDD =
client.upsertPreppedRecords(jsc.parallelize(filesPartitionExpectedRecords),
commitTimeOfInterest, Option.of(filesPartitionFileGroupIdList));
List<WriteStatus> filesPartitionWriteStatus =
filePartitionWriteStatusesRDD.collect();
+ // assert that isMetadata is rightly set
Review Comment:
```suggestion
// assert that `updatesMetadataTable` is set
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/WriteStatus.java:
##########
@@ -76,16 +78,22 @@ public class WriteStatus implements Serializable {
private final boolean trackSuccessRecords;
private final transient Random random;
- public WriteStatus(Boolean trackSuccessRecords, Double failureFraction) {
+ public WriteStatus(Boolean trackSuccessRecords, Double failureFraction,
Boolean updatesMetadataTable) {
Review Comment:
The `WriteStatus` class can be extended by user and plugged in through
`hoodie.writestatus.class`. Does MDT need such flexibility, i.e., should MDT
write status be fixated to use `WriteStatus`? Also, the change of constructor
here breaks compatibility (which is a minor issue if we add such breaking
change to release docs).
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieWriteStat.java:
##########
@@ -114,6 +119,10 @@ public class HoodieWriteStat extends HoodieReadStats {
@Nullable
private RuntimeStats runtimeStats;
+ @JsonIgnore
+ @AvroIgnore
+ private Option<Map<String, HoodieColumnRangeMetadata<Comparable>>>
recordsStats = Option.empty();
Review Comment:
Should `HoodieDeltaWriteStat` still serialize the record stats to the commit
metadata on storage if streaming write DAG is not enabled for backwards
compatibility?
--
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]