Fixing bug introducted in rollback for MOR table type with inserts into log files #417
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
n3nash
changed the title
|
@vinothchandar @bvaradar please review. |
bvaradar
reviewed
Jul 13, 2018
| super.deleteCleanedFiles(filesToDeletedStatus, partitionPath, filter); | ||
| final Set<String> deletedFiles = filesToDeletedStatus.entrySet().stream() | ||
| .map(entry -> { | ||
| Path filePath = entry.getKey().getPath(); |
There was a problem hiding this comment.
Minor : Looks like a reusable functionality of finding the fileId from a file-path (either log-file or parquet). Can you introduce a FSUtils method and move the logic there ?
| @@ -428,14 +429,16 @@ public void testRollbackWithDeltaAndCompactionCommit() throws Exception { | |||
| dataFilesToRead.findAny().isPresent()); | |||
|
|
|||
| /** | |||
| * Write 2 (updates) | |||
| * Write 2 (inserts + updates) | |||
There was a problem hiding this comment.
I tried applying the test-code patch alone to understand. I expected the test case to fail since I did not apply the actual fix but the test passed. Can you cross-check to see if some assertion needs to be added.
n3nash
force-pushed
the
bug_fix_for_rollback_inserts_into_log_files
branch
from
b29a807 to
51da8d4
Compare
|
@bvaradar Addressed your concerns, please let @vinothchandar know if it's ready to merge. |
n3nash
force-pushed
the
bug_fix_for_rollback_inserts_into_log_files
branch
from
51da8d4 to
ffb7077
Compare
vinishjail97
pushed a commit
to vinishjail97/hudi
that referenced
this pull request
Dec 15, 2023
Co-authored-by: StreamingFlames <18889897088@163.com>
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The bug was introduced here : 3da063f#diff-4c9f153416cce4b19fd73b4f1dcbb1d1R219
Essentially, the following situation takes place for MOR without inserts into log files :
In case this was a failed commit :
In case of a successful commit being rolled back :
The issue was there was no test case for the case when there are inserts + updates. The filter logic filtering out file ids from the workload for only updates was broken. The filter is now corrected.