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/11/28 21:35:25 UTC
[PR] [HUDI-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
codope opened a new pull request, #12369:
URL: https://github.com/apache/hudi/pull/12369
### Change Logs
Indexing catchup task iterates over all the commits to sync them to MDT. It will periodically check if the commit completed. But if the commit actually failed then it will forever be in inflight mode and hence the indexing run will not progress. This PR fixes the issue by using heartbeat to detect failed commits and ignore them.
### Impact
Indexer can be used even with pending/failed 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. If not, put "none"._
- _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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862749176
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/IndexingCatchupTaskFactory.java:
##########
@@ -36,11 +38,14 @@ public static IndexingCatchupTask createCatchupTask(List<HoodieIndexPartitionInf
HoodieTableMetadataWriter metadataWriter,
List<HoodieInstant> instantsToIndex,
Set<String> metadataCompletedInstants,
- HoodieTableMetaClient metaClient,
+ HoodieTable table,
HoodieTableMetaClient metadataMetaClient,
String currentCaughtupInstant,
TransactionManager transactionManager,
HoodieEngineContext engineContext) {
+ HoodieTableMetaClient metaClient = table.getMetaClient();
+ HoodieHeartbeatClient heartbeatClient = new HoodieHeartbeatClient(table.getStorage(), metaClient.getBasePath().toString(), table.getConfig().getHoodieClientHeartbeatIntervalInMs(),
Review Comment:
unless we do heartbeatClient.start(instantTime), no heart beats will be emitted.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "vinothchandar (via GitHub)" <gi...@apache.org>.
vinothchandar commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862725513
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
please dont return `null`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/IndexingCatchupTaskFactory.java:
##########
@@ -36,11 +38,14 @@ public static IndexingCatchupTask createCatchupTask(List<HoodieIndexPartitionInf
HoodieTableMetadataWriter metadataWriter,
List<HoodieInstant> instantsToIndex,
Set<String> metadataCompletedInstants,
- HoodieTableMetaClient metaClient,
+ HoodieTable table,
HoodieTableMetaClient metadataMetaClient,
String currentCaughtupInstant,
TransactionManager transactionManager,
HoodieEngineContext engineContext) {
+ HoodieTableMetaClient metaClient = table.getMetaClient();
+ HoodieHeartbeatClient heartbeatClient = new HoodieHeartbeatClient(table.getStorage(), metaClient.getBasePath().toString(), table.getConfig().getHoodieClientHeartbeatIntervalInMs(),
Review Comment:
would this just check for heartbeats..or actually do any heartbeats.. i.e write heartbeats.. ? just making sure
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2507806574
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d3c2dec84c3ad532f17eb2121ba44a8601ead8e Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074)
* cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f 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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862750528
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
btw, should we call `heartbeatExists`, followed by `isHeartbeatExpired` with the heart beat client, rather than directly calling isHeartbeatExpired.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan merged PR #12369:
URL: https://github.com/apache/hudi/pull/12369
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2507930389
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2506777322
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d3c2dec84c3ad532f17eb2121ba44a8601ead8e Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1863516660
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/IndexingCatchupTaskFactory.java:
##########
@@ -36,11 +38,14 @@ public static IndexingCatchupTask createCatchupTask(List<HoodieIndexPartitionInf
HoodieTableMetadataWriter metadataWriter,
List<HoodieInstant> instantsToIndex,
Set<String> metadataCompletedInstants,
- HoodieTableMetaClient metaClient,
+ HoodieTable table,
HoodieTableMetaClient metadataMetaClient,
String currentCaughtupInstant,
TransactionManager transactionManager,
HoodieEngineContext engineContext) {
+ HoodieTableMetaClient metaClient = table.getMetaClient();
+ HoodieHeartbeatClient heartbeatClient = new HoodieHeartbeatClient(table.getStorage(), metaClient.getBasePath().toString(), table.getConfig().getHoodieClientHeartbeatIntervalInMs(),
Review Comment:
yes, moved there.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2508166164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
}, {
"hash" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092)
* 7c5827ee463a0e760d6eb66c329a7b11e9cba569 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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862760556
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
let me summarize my understanding.
I also went over the design in the RFC https://github.com/apache/hudi/blob/master/rfc/rfc-45/rfc-45.md as well.
```
6 Start indexing catchup in a separate thread (that can be interrupted upon timeout)
6.a For each instant to catchup
6.a.i if instant is completed and has corresponding deltacommit in metadata timeline then continue
6.a.ii if instant is inflight, then reload active timeline periodically until completed or timed out
6.a.iii update metadata table, if needed, within a lock
```
so, in this case, just that we missed to account for a failed write scenario.
a. for a single writer, we should ignore the instant to catch up and continue in the loop to next instant.
b. for multi-writer scenario, if there is active heart beat, we have to wait. so that it either goes to completion. or it elapses time out on which case we can again ignore from the list of instant to catchup.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1863519008
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
I can't recall why it was this way but no use, so now i have changed it to boolean and heartbeat check is as follows:
1. If no heart beat, then ignore and just move on.
2. If heart beat exists, but expire, then also ignore and move on. If not expired then wait.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2506776512
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d3c2dec84c3ad532f17eb2121ba44a8601ead8e 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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2507808956
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d3c2dec84c3ad532f17eb2121ba44a8601ead8e Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074)
* cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2508285021
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
}, {
"hash" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2098",
"triggerID" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c5827ee463a0e760d6eb66c329a7b11e9cba569 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2098)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2508167937
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
}, {
"hash" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092",
"triggerID" : "cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f",
"triggerType" : "PUSH"
}, {
"hash" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2098",
"triggerID" : "7c5827ee463a0e760d6eb66c329a7b11e9cba569",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cc9d67ada7eb1706fab9b3cfcd13efc0030cd50f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2092)
* 7c5827ee463a0e760d6eb66c329a7b11e9cba569 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2098)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12369:
URL: https://github.com/apache/hudi/pull/12369#issuecomment-2506832575
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074",
"triggerID" : "8d3c2dec84c3ad532f17eb2121ba44a8601ead8e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d3c2dec84c3ad532f17eb2121ba44a8601ead8e Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2074)
<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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862763143
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/IndexingCatchupTaskFactory.java:
##########
@@ -36,11 +38,14 @@ public static IndexingCatchupTask createCatchupTask(List<HoodieIndexPartitionInf
HoodieTableMetadataWriter metadataWriter,
List<HoodieInstant> instantsToIndex,
Set<String> metadataCompletedInstants,
- HoodieTableMetaClient metaClient,
+ HoodieTable table,
HoodieTableMetaClient metadataMetaClient,
String currentCaughtupInstant,
TransactionManager transactionManager,
HoodieEngineContext engineContext) {
+ HoodieTableMetaClient metaClient = table.getMetaClient();
+ HoodieHeartbeatClient heartbeatClient = new HoodieHeartbeatClient(table.getStorage(), metaClient.getBasePath().toString(), table.getConfig().getHoodieClientHeartbeatIntervalInMs(),
Review Comment:
should we create the heartbeat in the constructor of RunIndexActionExecutor.
which is where transaction manager is created and passed along
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
and yes. same comment as vc.
awaitInstantCaughtUp(HoodieInstant instant) does not change the instant anyways. So, why not return boolean.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862749176
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/IndexingCatchupTaskFactory.java:
##########
@@ -36,11 +38,14 @@ public static IndexingCatchupTask createCatchupTask(List<HoodieIndexPartitionInf
HoodieTableMetadataWriter metadataWriter,
List<HoodieInstant> instantsToIndex,
Set<String> metadataCompletedInstants,
- HoodieTableMetaClient metaClient,
+ HoodieTable table,
HoodieTableMetaClient metadataMetaClient,
String currentCaughtupInstant,
TransactionManager transactionManager,
HoodieEngineContext engineContext) {
+ HoodieTableMetaClient metaClient = table.getMetaClient();
+ HoodieHeartbeatClient heartbeatClient = new HoodieHeartbeatClient(table.getStorage(), metaClient.getBasePath().toString(), table.getConfig().getHoodieClientHeartbeatIntervalInMs(),
Review Comment:
to VC: unless we do heartbeatClient.start(instantTime), no heart beats will be emitted.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1862750528
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -155,6 +164,15 @@ HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
return null;
}
if (!instant.isCompleted()) {
+ // check if instant heartbeat expired, if so then ignore this instant
+ try {
+ if (table.getConfig().getFailedWritesCleanPolicy().isLazy() && heartbeatClient.isHeartbeatExpired(instant.requestedTime())) {
+ LOG.info("Ignoring instant " + instant + " as heartbeat expired");
+ return null;
Review Comment:
btw, we should not be directly calling isHeartbeatExpired.
We should be calling `heartbeatExists`, followed by `isHeartbeatExpired` with the heart beat client.
in case of a single writer model,
```
public boolean isHeartbeatExpired(String instantTime) throws IOException {
Long currentTime = System.currentTimeMillis();
Heartbeat lastHeartbeatForWriter = instantToHeartbeatMap.get(instantTime);
if (lastHeartbeatForWriter == null) {
LOG.info("Heartbeat not found in internal map, falling back to reading from DFS");
long lastHeartbeatForWriterTime = getLastHeartbeatTime(this.storage, basePath, instantTime);
lastHeartbeatForWriter = new Heartbeat();
lastHeartbeatForWriter.setLastHeartbeatTime(lastHeartbeatForWriterTime);
lastHeartbeatForWriter.setInstantTime(instantTime);
lastHeartbeatForWriter.getTimer().cancel();
}
if (currentTime - lastHeartbeatForWriter.getLastHeartbeatTime() > this.maxAllowableHeartbeatIntervalInMs) {
LOG.warn("Heartbeat expired, currentTime = " + currentTime + ", last heartbeat = " + lastHeartbeatForWriter
+ ", heartbeat interval = " + this.heartbeatIntervalInMs);
return true;
}
return false;
}
```
we might unintentionally start the heart beat.
--
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-6371] Indexing catchup tasks should handle failed commits based on heartbeat [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #12369:
URL: https://github.com/apache/hudi/pull/12369#discussion_r1863749522
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/AbstractIndexingCatchupTask.java:
##########
@@ -147,22 +155,37 @@ public void run() {
* If not, it waits until the instant is completed.
*
* @param instant HoodieInstant to check
- * @return null if instant is already caught up, else the instant after it is completed.
+ * @return True if instant is already caught up, or no heartbeat, or expired heartbeat. If heartbeat exists and not expired, then return false.
*/
- HoodieInstant awaitInstantCaughtUp(HoodieInstant instant) {
+ boolean awaitInstantCaughtUp(HoodieInstant instant) {
if (!metadataCompletedInstants.isEmpty() && metadataCompletedInstants.contains(instant.requestedTime())) {
currentCaughtupInstant = instant.requestedTime();
- return null;
+ return true;
}
if (!instant.isCompleted()) {
+ // check heartbeat
+ try {
+ // if no heartbeat, then ignore this instant
+ if (!HoodieHeartbeatClient.heartbeatExists(metaClient.getStorage(), metaClient.getBasePath().toString(), instant.requestedTime())) {
Review Comment:
can we add java docs here.
essentially the notes that we discussed.
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/index/TestIndexingCatchupTask.java:
##########
@@ -135,6 +171,41 @@ public void testTaskInterrupted() {
}
}
+ /**
+ * Test case to cover heartbeat expiry. Validate that awaitInstantCaughtUp
+ * returns null when heartbeat has expired for the given instant.
+ */
+ @Test
+ public void testHeartbeatExpired() throws IOException {
+ HoodieInstant expiredInstant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.REQUESTED, "commit", "002");
+ HoodieWriteConfig writeConfig = HoodieWriteConfig.newBuilder()
+ .withPath("/some/path")
+ .withCleanConfig(HoodieCleanConfig.newBuilder().withFailedWritesCleaningPolicy(HoodieFailedWritesCleaningPolicy.LAZY).build())
+ .build();
+ // Simulate heartbeat exists and expired
+ when(table.getConfig()).thenReturn(writeConfig);
+ HoodieStorage storage = mock(HoodieStorage.class);
+ when(metaClient.getStorage()).thenReturn(storage);
+ when(metaClient.getBasePath()).thenReturn(new StoragePath("/some/path"));
+ when(storage.exists(any())).thenReturn(true);
+ when(heartbeatClient.isHeartbeatExpired("002")).thenReturn(true);
Review Comment:
do we have a case where heart beat does not exist?
--
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