You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "codope (via GitHub)" <gi...@apache.org> on 2024/02/15 15:06:17 UTC
[PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
codope opened a new pull request, #10676:
URL: https://github.com/apache/hudi/pull/10676
### Change Logs
Cleaner could not delete partitions but meta sync fails to drop partition in that case. This could cause query using engines that depend on catalog to fail.
TODO: I have only tested locally. I am going to add a test.
### Impact
Catalog will reflect correct partition metadata considering cleaner commits.
### 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_
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1985431672
@nsivabalan Added a test, please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1946386360
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d5f38b26cede75b6d07367cb661f0fd20256e3e0 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10676:
URL: https://github.com/apache/hudi/pull/10676#discussion_r1509371686
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -266,13 +266,19 @@ public static HoodieDefaultTimeline getTimeline(HoodieTableMetaClient metaClient
* @return Hudi timeline.
*/
public static HoodieTimeline getCommitsTimelineAfter(
- HoodieTableMetaClient metaClient, String exclusiveStartInstantTime, Option<String> lastMaxCompletionTime) {
+ HoodieTableMetaClient metaClient,
+ String exclusiveStartInstantTime,
+ Option<String> lastMaxCompletionTime,
+ boolean shouldIncludeCleanerTimeline) {
Review Comment:
Typically we do not consider the cleaner as part of the commits since it is not changing the data. Is the issue here that the dropped partition from the replace commit is not being picked up when looking at the partitions affected for the meta sync?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1985433841
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
}, {
"hash" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d5f38b26cede75b6d07367cb661f0fd20256e3e0 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457)
* 2433556442faea975ed3c97a0abb0a8adf2610c8 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10676:
URL: https://github.com/apache/hudi/pull/10676#discussion_r1518111949
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -266,13 +266,19 @@ public static HoodieDefaultTimeline getTimeline(HoodieTableMetaClient metaClient
* @return Hudi timeline.
*/
public static HoodieTimeline getCommitsTimelineAfter(
- HoodieTableMetaClient metaClient, String exclusiveStartInstantTime, Option<String> lastMaxCompletionTime) {
+ HoodieTableMetaClient metaClient,
+ String exclusiveStartInstantTime,
+ Option<String> lastMaxCompletionTime,
+ boolean shouldIncludeCleanerTimeline) {
Review Comment:
its not the lack of files. but cleaner deleted the physical partition dir, but we missed to sync to metastore and hence. but I see your point too.
your suggestion is to reverse the ordering. if it goes into replace commit, first we will sync to metastore to drop the partition and later lazily cleaner will take care of deleting the dir physically.
But w/ current approach, just after cleaner runs(after this fix), and before we run meta sync, if someone is querying, we might run into issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1946401504
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d5f38b26cede75b6d07367cb661f0fd20256e3e0 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #10676:
URL: https://github.com/apache/hudi/pull/10676#discussion_r1517529187
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -266,13 +266,19 @@ public static HoodieDefaultTimeline getTimeline(HoodieTableMetaClient metaClient
* @return Hudi timeline.
*/
public static HoodieTimeline getCommitsTimelineAfter(
- HoodieTableMetaClient metaClient, String exclusiveStartInstantTime, Option<String> lastMaxCompletionTime) {
+ HoodieTableMetaClient metaClient,
+ String exclusiveStartInstantTime,
+ Option<String> lastMaxCompletionTime,
+ boolean shouldIncludeCleanerTimeline) {
Review Comment:
We do consider dropped partitions due to replacecommit in `TimelineUtils.getDroppedPartitions`. What was happening is that replacecommit does not add a new filegroup to a partition when all records are deleted. Older filegroups are not active and then subsequently cleaner thinks there are no active filegroups in the partition and marks the partition to be deleted. https://github.com/apache/hudi/blob/58bc859b173a3648ff5f7f2042aaadf8281cac2c/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java#L353-L359
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10676:
URL: https://github.com/apache/hudi/pull/10676#discussion_r1518082368
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -266,13 +266,19 @@ public static HoodieDefaultTimeline getTimeline(HoodieTableMetaClient metaClient
* @return Hudi timeline.
*/
public static HoodieTimeline getCommitsTimelineAfter(
- HoodieTableMetaClient metaClient, String exclusiveStartInstantTime, Option<String> lastMaxCompletionTime) {
+ HoodieTableMetaClient metaClient,
+ String exclusiveStartInstantTime,
+ Option<String> lastMaxCompletionTime,
+ boolean shouldIncludeCleanerTimeline) {
Review Comment:
Does it make sense to mark those partitions as deleted when the replace commit happens though or only when the cleaner kicks in? The benefit of doing it when the replace commit happens is that it will kick in before the files are actually removed. I'm guessing that the lack of files was causing some issue in a query engine. If that is the case then you will have some downtime with the current approach of doing this after the cleaner has removed the files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1985530919
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
}, {
"hash" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22848",
"triggerID" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2433556442faea975ed3c97a0abb0a8adf2610c8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22848)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan merged PR #10676:
URL: https://github.com/apache/hudi/pull/10676
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10676:
URL: https://github.com/apache/hudi/pull/10676#discussion_r1518112714
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -266,13 +266,19 @@ public static HoodieDefaultTimeline getTimeline(HoodieTableMetaClient metaClient
* @return Hudi timeline.
*/
public static HoodieTimeline getCommitsTimelineAfter(
- HoodieTableMetaClient metaClient, String exclusiveStartInstantTime, Option<String> lastMaxCompletionTime) {
+ HoodieTableMetaClient metaClient,
+ String exclusiveStartInstantTime,
+ Option<String> lastMaxCompletionTime,
+ boolean shouldIncludeCleanerTimeline) {
Review Comment:
Yes that is my concern.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1985444564
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
}, {
"hash" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22848",
"triggerID" : "2433556442faea975ed3c97a0abb0a8adf2610c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d5f38b26cede75b6d07367cb661f0fd20256e3e0 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457)
* 2433556442faea975ed3c97a0abb0a8adf2610c8 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22848)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7411] Meta sync should consider cleaner commit [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10676:
URL: https://github.com/apache/hudi/pull/10676#issuecomment-1946813984
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457",
"triggerID" : "d5f38b26cede75b6d07367cb661f0fd20256e3e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d5f38b26cede75b6d07367cb661f0fd20256e3e0 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22457)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org