-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[HUDI-5078] Fixing determination of table service for metadata calls #7037
[HUDI-5078] Fixing determination of table service for metadata calls #7037
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.
lgtm. Have a comment on the API.
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java
Outdated
Show resolved
Hide resolved
...client/hudi-client-common/src/main/java/org/apache/hudi/table/action/BaseActionExecutor.java
Outdated
Show resolved
Hide resolved
...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java
Outdated
Show resolved
Hide resolved
b2d84de
to
40a2546
Compare
@xushiyan : feel free to take another look. |
40a2546
to
6b24d08
Compare
6b24d08
to
2ce44c5
Compare
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java
Show resolved
Hide resolved
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.
There is no need to construct the instant everywhere.
2ce44c5
to
c3cf14a
Compare
@danny0405 :
So, have kept it just for that use-case. have removed elsehwere. |
@xushiyan @danny0405 : addressed all reviews |
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.
only 1 suggestion for code improvement
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstant.java
Show resolved
Hide resolved
c3cf14a
to
afb7e32
Compare
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.
+1
Change Logs
While applying commit metadata to metadata table, there is an argument "isTableService" to determine whether the triggering action is a table service or not. We mistakenly considered any replace commit as table service. Right fix is, only clustering is table service and other replace commits are not.
Impact
Metadata compaction will make progress with replace commit actions like INSERT_OVERRIDE after this patch.
Risk level (write none, low medium or high below)
low.
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
N/A
Contributor's checklist