-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-6543] Enhance OCC conflict detection #9233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zouxxyy I am yet to review the patch fully. Just curious, why removing this strategy? Is it taken care of in the new conflict resolver based on state transition time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zouxxyy I am yet to review the patch fully. Just curious, why removing this strategy? Is it taken care of in the new conflict resolver based on state transition time?
Two reasons:
- If we want to use bucket index, ingestion primary, and don't want to use state transition time (because it is not stable yet), how to choose the strategy, should we write another one?
- I hope that the current strategy can automatically identify the bucket index and ingestion primary, which is better for users.
| if (!isIngestionPrimaryClustering()) { | ||
| actionsToPick.add(REPLACE_COMMIT_ACTION); | ||
| } | ||
| actionsToPick.add(COMPACTION_ACTION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I just keep the current implementation, because log compaction has pre commit now, hope to get more opinions @suryaprasanna
Change Logs
ConflictResolutionStrategyinterfaceSimpleConcurrentFileWritesConflictResolutionStrategyto automatically identify write priority or bucket index, and optimize the logic ofgetCandidateInstantsStateTransitionTimeBasedConflictResolutionStrategy, which is based on StateTransition and does not need to get pending instantslastCompletedTxnOwnerInstantinTransactionManager(only used for conflict detection, under the new logic, it has no effect)To know more, can read #9220
Impact
above
Risk level (write none, low medium or high below)
medium
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist