[HUDI-1434] fix incorrect log file path in HoodieWriteStat #2300

garyli1019 · 2020-12-05T13:02:29Z

What is the purpose of the pull request

Bugfix. Examples in the Jira: https://issues.apache.org/jira/browse/HUDI-1434

#1567 move the logic to determine whether we need to roll over the log file version when actually writing the file, but we write the log path to HoodieWriteStat when initializing the HoodieAppendHandle. If the log file is not appendable, the log file will roll over to the next version, but the HoodieWriteStat will still have the previous version, and this will make the deltacommit metadata has the wrong path.

Brief change log

Modified how we store the file path in HoodieAppendHandle

Verify this pull request

(Please pick either of the following options)

Tests will be covered by #1938

Committer checklist

Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

codecov-io · 2020-12-05T13:08:17Z

Codecov Report

Merging #2300 (150d183) into master (e807bb8) will decrease coverage by 42.16%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master    #2300       +/-   ##
=============================================
- Coverage     52.20%   10.04%   -42.17%     
+ Complexity     2659       48     -2611     
=============================================
  Files           335       52      -283     
  Lines         14981     1852    -13129     
  Branches       1505      223     -1282     
=============================================
- Hits           7821      186     -7635     
+ Misses         6535     1653     -4882     
+ Partials        625       13      -612

Flag	Coverage Δ	Complexity Δ
hudicli	`?`	`?`
hudiclient	`?`	`?`
hudicommon	`?`	`?`
hudihadoopmr	`?`	`?`
huditimelineservice	`?`	`?`
hudiutilities	`10.04% <ø> (-59.62%)`	`0.00 <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
...va/org/apache/hudi/utilities/IdentitySplitter.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-2.00%)`
...va/org/apache/hudi/utilities/schema/SchemaSet.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-3.00%)`
...a/org/apache/hudi/utilities/sources/RowSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
.../org/apache/hudi/utilities/sources/AvroSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-1.00%)`
.../org/apache/hudi/utilities/sources/JsonSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-1.00%)`
...rg/apache/hudi/utilities/sources/CsvDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-10.00%)`
...g/apache/hudi/utilities/sources/JsonDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
...apache/hudi/utilities/sources/JsonKafkaSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-6.00%)`
...pache/hudi/utilities/sources/ParquetDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-5.00%)`
...lities/schema/SchemaProviderWithPostProcessor.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
... and 307 more

vinothchandar · 2020-12-11T08:59:01Z

@n3nash could you please review this?

vinothchandar · 2020-12-13T22:32:47Z

@prashantwason this is an issue that will hit metadata table as well when rollovers happen. We did not see this thus far in tests since it was using hdfs which supports appends. Once I switched to local FS, I can see this issue actually

cc @n3nash

vinothchandar · 2020-12-13T22:45:41Z

@garyli1019 I think we should return a List from the append handle to be safe, given we can rollover the logs multiple times even for the same write ?

garyli1019 · 2020-12-14T03:23:01Z

@garyli1019 I think we should return a List from the append handle to be safe, given we can rollover the logs multiple times even for the same write ?

@vinothchandar maybe save the base file path as well? This could be useful for incremental pulling.

vinothchandar · 2020-12-15T06:05:15Z

@garyli1019 yes. thats also a good idea. we can have one entry per new log file created ?

vinothchandar · 2020-12-16T04:16:46Z

@garyli1019 this is a release blocker for 0.7.0. Do you have cycles to knock this off in the next few days? Please let me know if you need any help

garyli1019 · 2020-12-16T04:37:37Z

@garyli1019 this is a release blocker for 0.7.0. Do you have cycles to knock this off in the next few days? Please let me know if you need any help

@vinothchandar I will try this work on this PR over this weekend. If I can't finish, I will ask for help.

vinothchandar · 2020-12-16T05:47:01Z

Thanks @garyli1019

garyli1019 · 2020-12-19T12:11:53Z

@vinothchandar this is how the .deltacommit file looks like, please review. Thanks!

vinothchandar · 2020-12-20T17:32:06Z

@garyli1019 Thanks, will review tonight

vinothchandar

Thanks for quick turnaround! Have some clarifying comments.
So, in the json
path : is the latest log file written
baseFile : is the base file for the write or empty string.
logFiles : is the list of all log files, including the new one written

is my understanding correct? I think we need to handle the case where more than 1 log file was written during this write?

vinothchandar · 2020-12-21T23:58:23Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

      if (fileSlice.isPresent()) {
        baseInstantTime = fileSlice.get().getBaseInstantTime();
+        baseFile = fileSlice.get().getBaseFile().map(BaseFile::getFileName).orElse("");
+        logFiles = fileSlice.get().getLogFiles().map(HoodieLogFile::getFileName).collect(Collectors.toList());
      } else {
        baseInstantTime = instantTime;
        // This means there is no base data file, start appending to a new log file
        fileSlice = Option.of(new FileSlice(partitionPath, baseInstantTime, this.fileId));
        LOG.info("New InsertHandle for partition :" + partitionPath);


while we are at it. fix this log statement? New AppendHandle for ..

vinothchandar · 2020-12-22T08:37:29Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

-      writeStatus.getStat().setFileId(fileId);
+      deltaWriteStat.setPartitionPath(partitionPath);
+      deltaWriteStat.setFileId(fileId);
+      deltaWriteStat.setBaseFile(baseFile);


so this sets the baseFile and the logFiles that existed before this delta commit?

right, since we got this information from the flieSlice

vinothchandar · 2020-12-22T08:41:48Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

@@ -87,6 +88,9 @@
  private long averageRecordSize = 0;
  private HoodieLogFile currentLogFile;
  private Writer writer;
+  private String filePath = "null";
+  private int logVersion = 0;


in general., naming these variables with some context with some documentation, would help readability of this file.

e.g initialLogVersion , filePath -> logFilePathWritten etc

vinothchandar · 2020-12-22T08:43:39Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

      stat.setFileId(this.fileId);
+      stat.setPath(this.filePath);
+      stat.setLogVersion(logVersion);
+      stat.setLogOffset(logOffset);


this offset would still point to the file at which the write started?

this offset should be 0 if we write to a new log file, but the file size of the existing log file if we do append. Currently this offset seems not working because I am don't see anywhere this can get the log file size.

vinothchandar · 2020-12-22T08:44:41Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

@@ -303,6 +322,7 @@ private Writer createLogWriter(Option<FileSlice> fileSlice, String baseCommitTim
        .onParentPath(FSUtils.getPartitionPath(hoodieTable.getMetaClient().getBasePath(), partitionPath))
        .withFileId(fileId).overBaseCommit(baseCommitTime)
        .withLogVersion(latestLogFile.map(HoodieLogFile::getLogVersion).orElse(HoodieLogFile.LOGFILE_BASE_VERSION))
+        .withFileSize(latestLogFile.map(HoodieLogFile::getFileSize).orElse(0L))


why is this change needed?

trying to store the offset in the writer, so this will be sync with the log version. when the log version being changed, we can change the offset as well.

looks like I was wrong, in the picture, the log version increased but the offset is not 0. Will fix this

vinothchandar · 2020-12-22T08:46:29Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

      stat.setNumWrites(recordsWritten);
      stat.setNumUpdateWrites(updatedRecordsWritten);
      stat.setNumInserts(insertRecordsWritten);
      stat.setNumDeletes(recordsDeleted);
      stat.setTotalWriteBytes(estimatedNumberOfBytesWritten);
      stat.setFileSizeInBytes(sizeInBytes);
      stat.setTotalWriteErrors(writeStatus.getTotalErrorRecords());
+      // update total log file list if the latest log file was new
+      if (!stat.getLogFiles().contains(latestLogFile)) {


wht if we had rolled over the log files in between? i.e created more than one new log file during write. I think we should add to the list of log files, during every rollOver, not just at close

Not quite sure about the rollover case. Is that possible that two log files will be written in one commit? If we will write two log files, then the path should contain 2 files as well?

garyli1019 · 2020-12-22T13:40:42Z

Thanks for quick turnaround! Have some clarifying comments.
So, in the json
path : is the latest log file written
baseFile : is the base file for the write or empty string.
logFiles : is the list of all log files, including the new one written

is my understanding correct? I think we need to handle the case where more than 1 log file was written during this write?

yes, correct. Is there a case we will write more than 1 log file for a file group during a single commit?

garyli1019

@vinothchandar thanks for reviewing, left some questions need to be clarified.

garyli1019 · 2020-12-22T13:43:56Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

-      writeStatus.getStat().setFileId(fileId);
+      deltaWriteStat.setPartitionPath(partitionPath);
+      deltaWriteStat.setFileId(fileId);
+      deltaWriteStat.setBaseFile(baseFile);


right, since we got this information from the flieSlice

garyli1019 · 2020-12-22T13:48:46Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

      stat.setFileId(this.fileId);
+      stat.setPath(this.filePath);
+      stat.setLogVersion(logVersion);
+      stat.setLogOffset(logOffset);


this offset should be 0 if we write to a new log file, but the file size of the existing log file if we do append. Currently this offset seems not working because I am don't see anywhere this can get the log file size.

garyli1019 · 2020-12-22T13:51:42Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

      stat.setNumWrites(recordsWritten);
      stat.setNumUpdateWrites(updatedRecordsWritten);
      stat.setNumInserts(insertRecordsWritten);
      stat.setNumDeletes(recordsDeleted);
      stat.setTotalWriteBytes(estimatedNumberOfBytesWritten);
      stat.setFileSizeInBytes(sizeInBytes);
      stat.setTotalWriteErrors(writeStatus.getTotalErrorRecords());
+      // update total log file list if the latest log file was new
+      if (!stat.getLogFiles().contains(latestLogFile)) {


Not quite sure about the rollover case. Is that possible that two log files will be written in one commit? If we will write two log files, then the path should contain 2 files as well?

garyli1019 · 2020-12-22T14:03:10Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

@@ -303,6 +322,7 @@ private Writer createLogWriter(Option<FileSlice> fileSlice, String baseCommitTim
        .onParentPath(FSUtils.getPartitionPath(hoodieTable.getMetaClient().getBasePath(), partitionPath))
        .withFileId(fileId).overBaseCommit(baseCommitTime)
        .withLogVersion(latestLogFile.map(HoodieLogFile::getLogVersion).orElse(HoodieLogFile.LOGFILE_BASE_VERSION))
+        .withFileSize(latestLogFile.map(HoodieLogFile::getFileSize).orElse(0L))


trying to store the offset in the writer, so this will be sync with the log version. when the log version being changed, we can change the offset as well.

vinothchandar · 2020-12-22T16:35:22Z

yes, correct. Is there a case we will write more than 1 log file for a file group during a single commit?

@garyli1019 yes. certainly possible. We rollover log files once they pass a certain size. so if the amount of data is large, we will write more than 1.

I think we need to change WriteStatus to have a list of WriteStat . WDYT?

garyli1019 · 2020-12-23T05:31:48Z

I think we need to change WriteStatus to have a list of WriteStat . WDYT?

@vinothchandar IMO this will change how the commit metadata looks like and will affect many other places. Do you think we can handle this rollover case just in one WriteStat nicely?

vinothchandar · 2020-12-23T07:22:45Z

@garyli1019 if we write two log files, then we need a list somewhere , right? Not sure how we would deal with that. I mean either WriteStatus has List<WriteStat> or WriteStat has List<Path> for each file?

On complexity of the change, if we simply change the following instances

    List<HoodieWriteStat> writeStats = writeStatuses.map(WriteStatus::getStat).collect();

to

    List<HoodieWriteStat> writeStats = writeStatuses.flatMap(WriteStatus::getStats).collect();

it's simple enough right? commit logic may not change much? I have to take a deeper pass, but let me know if I am overlooking something big.

vinothchandar · 2020-12-23T07:23:14Z

cc @n3nash as well. since this is getting trickier.
cc @prashantwason as well. This is a blocker for RFC-15 on MOR

garyli1019 · 2020-12-23T08:04:01Z

it's simple enough right? commit logic may not change much? I have to take a deeper pass, but let me know if I am overlooking something big.

@vinothchandar Agree we should think deeper here. Right now we can assume one filegroup will have one writer and one WriteStats right?

garyli1019 · 2020-12-23T16:31:42Z

Don't wanna block the release here. Please feel free to push changes.

vinothchandar · 2020-12-23T19:21:03Z

Right now we can assume one filegroup will have one writer and one WriteStats right?

@garyli1019 yes . today WriteStatus has 1-1 mapping with a IO Handle operation.

May be the right thing is to return more WriteStatus objects. I will take a stab at this today and push 1 commit on top of your change. Let's discuss again in the PR.

vinothchandar · 2020-12-25T01:47:52Z

@garyli1019 I made some progress on this. Still some more work to do.

Key question: do you agree with representing a WriteStatus for each log file written. This is very consistent with what we have so far and plays well with Metadata table in a straightforward way

garyli1019 · 2020-12-25T02:45:09Z

Key question: do you agree with representing a WriteStatus for each log file written.

@vinothchandar Yes, if this way is consistent with Metadata table then we should do that. Thanks for driving this!

vinothchandar · 2020-12-26T16:37:43Z

@garyli1019 can you take a quick pass at this PR.
@n3nash is also going to review.

n3nash · 2020-12-27T03:56:58Z

@vinothchandar Changes look good, couple of comments after which can approve

vinothchandar · 2020-12-27T06:35:21Z

@n3nash I cannot see your comments. did you not submit the review or sth ?

garyli1019

@vinothchandar overall LGTM, left one comment.

garyli1019 · 2020-12-27T09:30:03Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

        .withSizeThreshold(config.getLogFileMaxSize()).withFs(fs)
-        .withLogWriteToken(latestLogFile.map(x -> FSUtils.getWriteTokenFromLogPath(x.getPath())).orElse(writeToken))


I am seeing the log version start from 2 in my local test. I guess it's probably we removed the write token here and initializing the token will increment the log version by 1. The HoodieLogFile.LOGFILE_BASE_VERSION is 1.

I will fix this. should not happen

n3nash · 2020-12-27T03:18:41Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

+
+  private void updateWriteStat(HoodieDeltaWriteStat stat, AppendResult result) {
+    stat.setPath(makeFilePath(result.logFile()));
+    stat.setLogOffset(result.offset());


Will this offset contain the offset of the last log file the append handle writes to ?

so the earliest offset it writes to . i.e the first time a log was written to by this append handle.

hudi-common/src/main/java/org/apache/hudi/common/model/HoodieDeltaWriteStat.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java

…itten - Combined data and delete block logging into a single call - Lazily initialize and manage write status based on returned AppendResult - Use FSUtils.getFileSize() to set final file size, consistent with other handles - Added tests around returned values in AppendResult - Added validation of the file sizes returned in write stat

vinothchandar · 2020-12-29T22:32:33Z

@garyli @n3nash added more tests. should be good to go now.

please take another pass and merge when CI is happy.

@garyli1019 are we now able to get the incremental/MOR spark datasource PR working based on this? by friday or so?

garyli1019 · 2020-12-30T02:19:57Z

@garyli1019 are we now able to get the incremental/MOR spark datasource PR working based on this? by friday or so?

LGTM, thanks for stepping in. Yes, I will update the incremental MOR PR based on this today.

n3nash

LGTM

* [HUDI-1434] fix incorrect log file path in HoodieWriteStat * HoodieWriteHandle#close() returns a list of WriteStatus objs * Handle rolled-over log files and return a WriteStatus per log file written - Combined data and delete block logging into a single call - Lazily initialize and manage write status based on returned AppendResult - Use FSUtils.getFileSize() to set final file size, consistent with other handles - Added tests around returned values in AppendResult - Added validation of the file sizes returned in write stat Co-authored-by: Vinoth Chandar <vinoth@apache.org>

…pache#2185) Summary: [MINOR] Make sure factory method is used to instanciate DFSPathSelector (apache#2187) * Move createSourceSelector into DFSPathSelector factory method * Replace constructor call with factory method * Added some javadoc [HUDI-1330] handle prefix filtering at directory level (apache#2157) The current DFSPathSelector only ignore prefix(_, .) at the file level while files under subdirectories e.g. (.checkpoint/*) are still considered which result in bad-format exception during reading. [HUDI-1200] fixed NPE in CustomKeyGenerator (apache#2093) - config field is no longer transient in key generator - verified that the key generator object is shipped from the driver to executors, just the one time and reused for each record [HUDI-1209] Properties File must be optional when running deltastreamer (apache#2085) [MINOR] Fix caller to SparkBulkInsertCommitActionExecutor (apache#2195) Fixed calling the wrong constructor [HUDI-1326] Added an API to force publish metrics and flush them. (apache#2152) * [HUDI-1326] Added an API to force publish metrics and flush them. Using the added API, publish metrics after each level of the DAG completed in hudi-test-suite. * Code cleanups Co-authored-by: Vinoth Chandar <vinoth@apache.org> [HUDI-1118] Cleanup rollback files residing in .hoodie folder (apache#2205) [MINOR] Private the NoArgsConstructor of SparkMergeHelper and code clean (apache#2194) 1. Fix merge on read DAG to make docker demo pass (apache#2092) 1. Fix merge on read DAG to make docker demo pass (apache#2092) 2. Fix repeat_count, rollback node [HUDI-1274] Make hive synchronization supports hourly partition (apache#2122) [HUDI-1351] Improvements to the hudi test suite for scalability and repeated testing. (apache#2197) 1. Added the --clean-input and --clean-output parameters to clean the input and output directories before starting the job 2. Added the --delete-old-input parameter to deleted older batches for data already ingested. This helps keep number of redundant files low. 3. Added the --input-parallelism parameter to restrict the parallelism when generating input data. This helps keeping the number of generated input files low. 4. Added an option start_offset to Dag Nodes. Without ability to specify start offsets, data is generated into existing partitions. With start offset, DAG can control on which partition, the data is to be written. 5. Fixed generation of records for correct number of partitions - In the existing implementation, the partition is chosen as a random long. This does not guarantee exact number of requested partitions to be created. 6. Changed variable blacklistedFields to be a Set as that is faster than List for membership checks. 7. Fixed integer division for Math.ceil. If two integers are divided, the result is not double unless one of the integer is casted to double. [HUDI-1338] Adding Delete support to test suite framework (apache#2172) - Adding Delete support to test suite. Added DeleteNode Added support to generate delete records Use RateLimiter instead of sleep. Repartition WriteStatus to optimize Hbase index writes (apache#1484) [HUDI-912] Refactor and relocate KeyGenerator to support more engines (apache#2200) * [HUDI-912] Refactor and relocate KeyGenerator to support more engines * Rename KeyGenerators [HUDI-892] RealtimeParquetInputFormat skip adding projection columns if there are no log files (apache#2190) * [HUDI-892] RealtimeParquetInputFormat skip adding projection columns if there are no log files * [HUDI-892] for test * [HUDI-892] fix bug generate array from split * [HUDI-892] revert test log [HUDI-1352] Add FileSystemView APIs to query pending clustering operations (apache#2202) [HUDI-1375] Fix bug in HoodieAvroUtils.removeMetadataFields() method (apache#2232) Co-authored-by: Wenning Ding <wenningd@amazon.com> [HUDI-1358] Fix Memory Leak in HoodieLogFormatWriter (apache#2217) [HUDI-1377] remove duplicate code (apache#2235) [HUDI-1327] Introduce base implemetation of hudi-flink-client (apache#2176) [HUDI-1400] Replace Operation enum with WriteOperationType (apache#2259) [HUDI-1384] Decoupling hive jdbc dependency when HIVE_USE_JDBC_OPT_KEY set false (apache#2241) [MINOR] clean up and add comments to flink client (apache#2261) [MINOR] Add apacheflink label (apache#2268) [HUDI-1393] Add compaction action in archive command (apache#2246) [HUDI-1364] Add HoodieJavaEngineContext to hudi-java-client (apache#2222) [HUDI-1396] Fix for preventing bootstrap datasource jobs from hanging via spark-submit (apache#2253) Co-authored-by: Wenning Ding <wenningd@amazon.com> [HUDI-1358] Fix leaks in DiskBasedMap and LazyFileIterable (apache#2249) [HUDI-1392] lose partition info when using spark parameter basePath (apache#2243) Co-authored-by: zhang wen <wen.zhang@dmall.com> [MINOR] refactor code in HoodieMergeHandle (apache#2272) [HUDI-1424] Write Type changed to BULK_INSERT when set ENABLE_ROW_WRITER_OPT_KEY=true (apache#2289) [HUDI-1373] Add Support for OpenJ9 JVM (apache#2231) * add supoort for OpenJ9 VM * add 32bit openJ9 * Pulled the memory layout specs into their own classes. [HUDI-1357] Added a check to validate records are not lost during merges. (apache#2216) - Turned off by default [HUDI-1196] Update HoodieKey when deduplicating records with global index (apache#2248) - Works only for overwrite payload (default) - Does not alter current semantics otherwise Co-authored-by: Ryan Pifer <ryanpife@amazon.com> [HUDI-1349] spark sql support overwrite use insert_overwrite_table (apache#2196) [HUDI-1343] Add standard schema postprocessor which would rewrite the schema using spark-avro conversion (apache#2192) Co-authored-by: liujh <liujh@t3go.cn> [HUDI-1427] Fix FileAlreadyExistsException when set HOODIE_AUTO_COMMIT_PROP to true (apache#2295) [HUDI-1412] Make HoodieWriteConfig support setting different default … (apache#2278) * [HUDI-1412] Make HoodieWriteConfig support setting different default value according to engine type fix typo (apache#2308) Co-authored-by: Xi Chen <chenxi07@qiyi.com> [HUDI-1040] Make Hudi support Spark 3 (apache#2208) * Fix flaky MOR unit test * Update Spark APIs to make it be compatible with both spark2 & spark3 * Refactor bulk insert v2 part to make Hudi be able to compile with Spark3 * Add spark3 profile to handle fasterxml & spark version * Create hudi-spark-common module & refactor hudi-spark related modules Co-authored-by: Wenning Ding <wenningd@amazon.com> [MINOR] Throw an exception when keyGenerator initialization failed (apache#2307) [HUDI-1395] Fix partition path using FSUtils (apache#2312) Fixed the logic to get partition path in Copier and Exporter utilities. [HUDI-1445] Refactor AbstractHoodieLogRecordScanner to use Builder (apache#2313) [MINOR] Minor improve in IncrementalRelation (apache#2314) [HUDI-1439] Remove scala dependency from hudi-client-common (apache#2306) [HUDI-1428] Clean old fileslice is invalid (apache#2292) Co-authored-by: zhang wen <wen.zhang@dmall.com> Co-authored-by: zhang wen <steven@stevendeMac-mini.local> [HUDI-1448] Hudi dla sync support skip rt table syncing (apache#2324) [HUDI-1435] Fix bug in Marker File Reconciliation for Non-Partitioned datasets (apache#2301) [MINOR] Improve code readability by passing in the fileComparisonsRDD in bloom index (apache#2319) [HUDI-1376] Drop Hudi metadata cols at the beginning of Spark datasource writing (apache#2233) Co-authored-by: Wenning Ding <wenningd@amazon.com> [MINOR] Fix error information in exception (apache#2341) [MINOR] Make QuickstartUtil generate random timestamp instead of 0 (apache#2340) [HUDI-1406] Add date partition based source input selector for Delta streamer (apache#2264) - Adds ability to list only recent date based partitions from source data. - Parallelizes listing for faster tailing of DFSSources [HUDI-1437] support more accurate spark JobGroup for better performance tracking (apache#2322) [HUDI-1470] Use the latest writer schema, when reading from existing parquet files in the hudi-test-suite (apache#2344) [HUDI-115] Adding DefaultHoodieRecordPayload to honor ordering with combineAndGetUpdateValue (apache#2311) * Added ability to pass in `properties` to payload methods, so they can perform table/record specific merges * Added default methods so existing payload classes are backwards compatible. * Adding DefaultHoodiePayload to honor ordering while merging two records * Fixing default payload based on feedback [HUDI-1419] Add base implementation for hudi java client (apache#2286) [MINOR] Pass root exception to HoodieKeyGeneratorException for more information (apache#2354) Co-authored-by: Xi Chen <chenxi07@qiyi.com> [HUDI-1075] Implement simple clustering strategies to create ClusteringPlan and to run the plan [HUDI-1471] Make QuickStartUtils generate deletes according to specific ts (apache#2357) [HUDI-1485] Fix Deletes issued without any prior commits exception (apache#2361) [HUDI-1488] Fix Test Case Failure in TestHBaseIndex (apache#2365) [HUDI-1489] Fix null pointer exception when reading updated written bootstrap table (apache#2370) Co-authored-by: Wenning Ding <wenningd@amazon.com> [HUDI-1451] Support bulk insert v2 with Spark 3.0.0 (apache#2328) Co-authored-by: Wenning Ding <wenningd@amazon.com> - Added support for bulk insert v2 with datasource v2 api in Spark 3.0.0. [HUDI-1487] fix unit test testCopyOnWriteStorage random failed (apache#2364) [HUDI-1490] Incremental Query should work even when there are partitions that have no incremental changes (apache#2371) * Incremental Query should work even when there are partitions that have no incremental changes Co-authored-by: Sivabalan Narayanan <sivabala@uber.com> [HUDI-1331] Adding support for validating entire dataset and long running tests in test suite framework (apache#2168) * trigger rebuild * [HUDI-1156] Remove unused dependencies from HoodieDeltaStreamerWrapper Class (apache#1927) * Adding support for validating records and long running tests in test sutie framework * Adding partial validate node * Fixing spark session initiation in Validate nodes * Fixing validation * Adding hive table validation to ValidateDatasetNode * Rebasing with latest commits from master * Addressing feedback * Addressing comments Co-authored-by: lamber-ken <lamberken@163.com> Co-authored-by: linshan-ma <mabin194046@163.com> [HUDI-1481] add structured streaming and delta streamer clustering unit test (apache#2360) [HUDI-1354] Block updates and replace on file groups in clustering (apache#2275) * [HUDI-1354] Block updates and replace on file groups in clustering * [HUDI-1354] Block updates and replace on file groups in clustering [HUDI-1350] Support Partition level delete API in HUDI (apache#2254) * [HUDI-1350] Support Partition level delete API in HUDI * [HUDI-1350] Support Partition level delete API in HUDI base InsertOverwriteCommitAction * [HUDI-1350] Support Partition level delete API in HUDI base InsertOverwriteCommitAction [HUDI-1495] Upgrade Flink version to 1.12.0 (apache#2384) [MINOR] Remove the duplicate code in AbstractHoodieWriteClient.startCommit (apache#2385) [HUDI-1398] Align insert file size for reducing IO (apache#2256) * [HUDI-1398] Align insert file size for reducing IO Co-authored-by: zhang wen <wen.zhang@dmall.com> [HUDI-1484] Escape the partition value in HiveSyncTool (apache#2363) [HUDI-1474] Add additional unit tests to TestHBaseIndex (apache#2349) [HUDI-1147] Modify GenericRecordFullPayloadGenerator to generate vali… (apache#2045) * [HUDI-1147] Modify GenericRecordFullPayloadGenerator to generate valid timestamps Co-authored-by: Sivabalan Narayanan <sivabala@uber.com> [HUDI-1493] Fixed schema compatibility check for fields. (apache#2350) Some field types changes are allowed (e.g. int -> long) while maintaining schema backward compatibility within HUDI. The check was reversed with the reader schema being passed for the write schema. [MINOR] Update report_coverage.sh (apache#2396) [HUDI-1434] fix incorrect log file path in HoodieWriteStat (apache#2300) * [HUDI-1434] fix incorrect log file path in HoodieWriteStat * HoodieWriteHandle#close() returns a list of WriteStatus objs * Handle rolled-over log files and return a WriteStatus per log file written - Combined data and delete block logging into a single call - Lazily initialize and manage write status based on returned AppendResult - Use FSUtils.getFileSize() to set final file size, consistent with other handles - Added tests around returned values in AppendResult - Added validation of the file sizes returned in write stat Co-authored-by: Vinoth Chandar <vinoth@apache.org> [HUDI-1418] Set up flink client unit test infra (apache#2281) [MINOR] Sync UpsertPartitioner modify of HUDI-1398 to flink/java (apache#2390) Co-authored-by: zhang wen <wen.zhang@dmall.com> [HUDI-1423] Support delete in hudi-java-client (apache#2353) [MINOR] Add maven profile to support skipping shade sources jars (apache#2358) Co-authored-by: Xi Chen <chenxi07@qiyi.com> [HUDI-842] Implementation of HUDI RFC-15. - Introduced an internal metadata table, that stores file listings. - metadata table is kept upto date with - Fixed handling of CleanerPlan. - [HUDI-842] Reduce parallelism to speed up the test. - [HUDI-842] Implementation of CLI commands for metadata operations and lookups. - [HUDI-842] Extend rollback metadata to include the files which have been appended to. - [HUDI-842] Support for rollbacks in MOR Table. - MarkerBasedRollbackStrategy needs to correctly provide the list of files for which rollback blocks were appended. - [HUDI-842] Added unit test for rollback of partial commits (inflight but not completed yet). - [HUDI-842] Handled the error case where metadata update succeeds but dataset commit fails. - [HUDI-842] Schema evolution strategy for Metadata Table. Each type of metadata saved (FilesystemMetadata, ColumnIndexMetadata, etc.) will be a separate field with default null. The type of the record will identify the valid field. This way, we can grow the schema when new type of information is saved within in which still keeping it backward compatible. - [HUDI-842] Fix non-partitioned case and speedup initial creation of metadata table.Choose only 1 partition for jsc as the number of records is low (hundreds to thousands). There is more overhead of creating large number of partitions for JavaRDD and it slows down operations like WorkloadProfile. For the non-partitioned case, use "." as the name of the partition to prevent empty keys in HFile. - [HUDI-842] Reworked metrics pusblishing. - Code has been split into reader and writer side. HoodieMetadata code to be accessed by using HoodieTable.metadata() to get instance of metdata for the table. Code is serializable to allow executors to use the functionality. - [RFC-15] Add metrics to track the time for each file system call. - [RFC-15] Added a distributed metrics registry for spark which can be used to collect metrics from executors. This helps create a stats dashboard which shows the metadata table improvements in real-time for production tables. - [HUDI-1321] Created HoodieMetadataConfig to specify configuration for the metadata table. This is safer than full-fledged properties for the metadata table (like HoodieWriteConfig) as it makes burdensome to tune the metadata. With limited configuration, we can control the performance of the metadata table closely. [HUDI-1319][RFC-15] Adding interfaces for HoodieMetadata, HoodieMetadataWriter (apache#2266) - moved MetadataReader to HoodieBackedTableMetadata, under the HoodieTableMetadata interface - moved MetadataWriter to HoodieBackedTableMetadataWriter, under the HoodieTableMetadataWriter - Pulled all the metrics into HoodieMetadataMetrics - Writer now wraps the metadata, instead of extending it - New enum for MetadataPartitionType - Streamlined code flow inside HoodieBackedTableMetadataWriter w.r.t initializing metadata state - [HUDI-1319] Make async operations work with metadata table (apache#2332) - Changes the syncing model to only move over completed instants on data timeline - Syncing happens postCommit and on writeClient initialization - Latest delta commit on the metadata table is sufficient as the watermark for data timeline archival - Cleaning/Compaction use a suffix to the last instant written to metadata table, such that we keep the 1-1 - .. mapping between data and metadata timelines. - Got rid of a lot of the complexity around checking for valid commits during open of base/log files - Tests now use local FS, to simulate more failure scenarios - Some failure scenarios exposed HUDI-1434, which is needed for MOR to work correctly co-authored by: Vinoth Chandar <vinoth@apache.org> [HUDI-1450] Use metadata table for listing in HoodieROTablePathFilter (apache#2326) [HUDI-1394] [RFC-15] Use metadata table (if present) to get all partition paths (apache#2351) [HUDI-1469] Faster initialization of metadata table using parallelized listing. (apache#2343) * [HUDI-1469] Faster initialization of metadata table using parallelized listing which finds partitions and files in a single scan. * MINOR fixes Co-authored-by: Vinoth Chandar <vinoth@apache.org> [HUDI-1325] [RFC-15] Merge updates of unsynced instants to metadata table (apache#2342) [RFC-15] Fix partition key in metadata table when bootstrapping from file system (apache#2387) Co-authored-by: Ryan Pifer <ryanpife@amazon.com> [HUDI-1312] [RFC-15] Support for metadata listing for snapshot queries through Hive/SparkSQL (apache#2366) Co-authored-by: Ryan Pifer <ryanpife@amazon.com> [HUDI-1504] Allow log files generated during restore/rollback to be synced as well - TestHoodieBackedMetadata#testSync etc now run for MOR tables - HUDI-1502 is still pending and has issues for MOR/rollbacks - Also addressed bunch of code review comments. [HUDI-1498] Read clustering plan from requested file for inflight instant (apache#2389) [HUDI-1506] Fix wrong exception thrown in HoodieAvroUtils (apache#2405) [HUDI-1383] Fixing sorting of partition vals for hive sync computation (apache#2402) [HUDI-1507] Change timeline utils to support reading replacecommit metadata (apache#2407) [MINOR] Rename unit test package of hudi-spark3 from scala to java (apache#2411) [HUDI-1513] Introduce WriteClient#preWrite() and relocate metadata table syncing (apache#2413) - Syncing to metadata table, setting operation type, starting async cleaner done in preWrite() - Fixes an issues where delete() was not starting async cleaner correctly - Fixed tests and enabled metadata table for TestAsyncCompaction [HUDI-1510] Move HoodieEngineContext and its dependencies to hudi-common (apache#2410) [MINOR] Sync HUDI-1196 to FlinkWriteHelper (apache#2415) [HUDI-1514] Avoid raw type use for parameter of Transformer interface (apache#2420) [HUDI-920] Support Incremental query for MOR table (apache#1938) [HUDI-1276] [HUDI-1459] Make Clustering/ReplaceCommit and Metadata table be compatible (apache#2422) * [HUDI-1276] [HUDI-1459] Make Clustering/ReplaceCommit and Metadata table be compatible * Use filesystemview and json format from metadata. Add tests Co-authored-by: Satish Kotha <satishkotha@uber.com> [HUDI-1399] support a independent clustering spark job to asynchronously clustering (apache#2379) * [HUDI-1481] add structured streaming and delta streamer clustering unit test * [HUDI-1399] support a independent clustering spark job to asynchronously clustering * [HUDI-1399] support a independent clustering spark job to asynchronously clustering * [HUDI-1498] Read clustering plan from requested file for inflight instant (apache#2389) * [HUDI-1399] support a independent clustering spark job with schedule generate instant time Co-authored-by: satishkotha <satishkotha@uber.com> [MINOR] fix spark 3 build for incremental query on MOR (apache#2425) [HUDI-1479] Use HoodieEngineContext to parallelize fetching of partiton paths (apache#2417) * [HUDI-1479] Use HoodieEngineContext to parallelize fetching of partition paths * Adding testClass for FileSystemBackedTableMetadata Co-authored-by: Nishith Agarwal <nagarwal@uber.com> [HUDI-1520] add configure for spark sql overwrite use INSERT_OVERWRITE_TABLE (apache#2428) [HUDI-1502] MOR rollback and restore support for metadata sync (apache#2421) - Adds field to RollbackMetadata that capture the logs written for rollback blocks - Adds field to RollbackMetadata that capture new logs files written by unsynced deltacommits Co-authored-by: Vinoth Chandar <vinoth@apache.org> Reviewers: O955 Project Hoodie Project Reviewer: Add blocking reviewers!, PHID-PROJ-pxfpotkfgkanblb3detq!, #ldap_hudi, modi Reviewed By: #ldap_hudi, modi Differential Revision: https://code.uberinternal.com/D5347141

garyli1019 requested review from vinothchandar and bvaradar December 5, 2020 13:03

garyli1019 force-pushed the HUDI-1434 branch 2 times, most recently from d4b29b9 to db85097 Compare December 5, 2020 15:36

vinothchandar assigned vinothchandar and n3nash Dec 11, 2020

vinothchandar mentioned this pull request Dec 13, 2020

[HUDI-1319] Make async operations work with metadata table #2332

Merged

5 tasks

garyli1019 force-pushed the HUDI-1434 branch from db85097 to a0b89be Compare December 19, 2020 12:03

garyli1019 force-pushed the HUDI-1434 branch from a0b89be to 7f375aa Compare December 19, 2020 13:05

vinothchandar reviewed Dec 22, 2020

View reviewed changes

garyli1019 commented Dec 22, 2020

View reviewed changes

garyli1019 added the priority:blocker label Dec 26, 2020

garyli1019 and others added 2 commits December 26, 2020 06:56

[HUDI-1434] fix incorrect log file path in HoodieWriteStat

a6c0331

HoodieWriteHandle#close() returns a list of WriteStatus objs

9c25633

vinothchandar force-pushed the HUDI-1434 branch 3 times, most recently from 2930bff to 207b046 Compare December 26, 2020 15:19

garyli1019 commented Dec 27, 2020

View reviewed changes

n3nash reviewed Dec 27, 2020

View reviewed changes

vinothchandar force-pushed the HUDI-1434 branch from 207b046 to 150d183 Compare December 29, 2020 22:30

n3nash approved these changes Dec 30, 2020

View reviewed changes

n3nash merged commit 605b617 into apache:master Dec 30, 2020

		.withSizeThreshold(config.getLogFileMaxSize()).withFs(fs)
		.withLogWriteToken(latestLogFile.map(x -> FSUtils.getWriteTokenFromLogPath(x.getPath())).orElse(writeToken))

[HUDI-1434] fix incorrect log file path in HoodieWriteStat #2300

[HUDI-1434] fix incorrect log file path in HoodieWriteStat #2300

Conversation

garyli1019 commented Dec 5, 2020

What is the purpose of the pull request

Brief change log

Verify this pull request

Committer checklist

codecov-io commented Dec 5, 2020 • edited

Codecov Report

vinothchandar commented Dec 11, 2020

vinothchandar commented Dec 13, 2020

vinothchandar commented Dec 13, 2020

garyli1019 commented Dec 14, 2020

vinothchandar commented Dec 15, 2020

vinothchandar commented Dec 16, 2020

garyli1019 commented Dec 16, 2020

vinothchandar commented Dec 16, 2020

garyli1019 commented Dec 19, 2020

vinothchandar commented Dec 20, 2020

vinothchandar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyli1019 commented Dec 22, 2020

garyli1019 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinothchandar commented Dec 22, 2020

garyli1019 commented Dec 23, 2020 • edited

vinothchandar commented Dec 23, 2020

vinothchandar commented Dec 23, 2020

garyli1019 commented Dec 23, 2020

garyli1019 commented Dec 23, 2020

vinothchandar commented Dec 23, 2020

vinothchandar commented Dec 25, 2020

garyli1019 commented Dec 25, 2020

vinothchandar commented Dec 26, 2020

n3nash commented Dec 27, 2020

vinothchandar commented Dec 27, 2020

garyli1019 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinothchandar commented Dec 29, 2020

garyli1019 commented Dec 30, 2020

n3nash left a comment

Choose a reason for hiding this comment

codecov-io commented Dec 5, 2020 •

edited

garyli1019 commented Dec 23, 2020 •

edited