[HUDI-1955]Fix the filter condition is missing in the judgment condition of comp… #3025
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3025 +/- ##
============================================
+ Coverage 45.75% 45.80% +0.04%
- Complexity 5262 5275 +13
============================================
Files 909 909
Lines 39356 39419 +63
Branches 4239 4250 +11
============================================
+ Hits 18008 18056 +48
- Misses 19504 19516 +12
- Partials 1844 1847 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Hi, @vinothchandar , can you take a look for this small fix, i'm confident that this may be a bug here ~ |
| @@ -63,7 +63,7 @@ public BaseScheduleCompactionActionExecutor(HoodieEngineContext context, | |||
| + ", Compaction scheduled at " + instantTime)); | |||
| // Committed and pending compaction instants should have strictly lower timestamps | |||
| List<HoodieInstant> conflictingInstants = table.getActiveTimeline() | |||
| .getWriteTimeline().getInstants() | |||
| .getWriteTimeline().filterCompletedAndCompactionInstants().getInstants() | |||
There was a problem hiding this comment.
@swuferhong @danny0405 If you take a look at the previous version of this file, the method is called before was commitsAndCompactionTimeline ->
This follows the same behavior as getWriteTimeline().getInstants(). Can you please explain what is a possible bug here ?
There was a problem hiding this comment.
No, the method getWriteTimeline() does not really follow the behavior of filterCompletedAndCompactionInstants,
getWriteTimeline() actually may include any INFLIGHT instants but filterCompletedAndCompactionInstants only include COMPACTION INFLIGHT instants.
We should allow scheduling compaction if there are inflight commits or inflight delta_commits.
There was a problem hiding this comment.
@danny0405 This class was NOT using filterCompletedAndCompactionInstants before. It was using commitsAndCompactionTimeline(). See this -> https://github.com/apache/hudi/blob/release-0.7.0/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/compact/BaseScheduleCompactionActionExecutor.java#L65
Let me know if there is any confusion.
There was a problem hiding this comment.
Take a look at the comments
// Committed and pending compaction instants should have strictly lower timestampsI think the code before that used commitsAndCompactionTimeline() is already wrong, it add restrictions that we can not generate compaction plan when there are inflight commits, of course we can actually.
There was a problem hiding this comment.
Okay got it. So you're saying this bug was always there even in 0.7 release ? Can we please add a test case for this before landing ?
There was a problem hiding this comment.
Sure, let us add a test case.
…action instance
Tips
What is the purpose of the pull request
The filter condition is missing in the judgment condition of compaction instance in BaseScheduleCompactionActionExecutor.java. In method execute(), it needs a filter condition, but now it don't have.
Brief change log
(for example:)
Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
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.