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]

Reply via email to