You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "the-other-tim-brown (via GitHub)" <gi...@apache.org> on 2024/11/18 01:46:27 UTC
[PR] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
the-other-tim-brown opened a new pull request, #12272:
URL: https://github.com/apache/hudi/pull/12272
### Change Logs
- Remove an extra timeline reload during conflict resolution
### Impact
- Reduce file listing during writes
### 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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #12272:
URL: https://github.com/apache/hudi/pull/12272
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1853184421
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
maybe `timelineRefreshed` to indicate whether the timeline is already the latest while invoking this method. I'm not good at naming though.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1845813738
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
It looks like `getCompletedInstantsDuringCurrentWriteOperation` always need a timeline reload, maybe we should just remove line 74 ~ line 76
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513265613
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
}, {
"hash" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200",
"triggerID" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"triggerType" : "PUSH"
}, {
"hash" : "7e6238a557de32deb0131929b630e423440f8ac6",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2202",
"triggerID" : "7e6238a557de32deb0131929b630e423440f8ac6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2be8602e16700000585e0e9899891b4aed6a19b6 Azure: [CANCELED](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200)
* 7e6238a557de32deb0131929b630e423440f8ac6 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2202)
<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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1846613292
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
Yes, as Vinoth has pointed out this is not required because it is reloaded within the lock already. That is why there is a parameter on the method.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847540434
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
Can you point out the line that "reload the timeline within the lock", if it is already realoded, what is the purpose for another reload?
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847604456
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestSimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -412,6 +412,7 @@ public void testConcurrentWritesWithPendingInstants() throws Exception {
// make sure c3 has conflict with C1,C11,C12,C4;
HoodieCommitMetadata currentMetadata = createCommitMetadata(currentWriterInstant, "file-2");
timeline.reload();
+ metaClient.reloadActiveTimeline();
List<HoodieInstant> completedInstantsDuringCurrentWriteOperation = TransactionUtils
Review Comment:
What's the difference between `timeline.reload();` and `metaClient.reloadActiveTimeline();`
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1853184421
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
maybe `timelineRefreshed` to indicate whether the timeline is already the latest while invoking this method.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847555196
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
There is an argument to this method called `reloadActiveTimeline` and it is true [here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/BaseCommitActionExecutor.java#L202-L203) since the table is created outside of the lock.
It is false [here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieClient.java#L239-L240). This method is called from the preCommit methods in [BaseHoodieWriteClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java#L378) and [BaseHoodieTableServiceClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java#L179). Both of these are creating the table instance which will create a new meta client right before calling the method linked in `BaseHoodieWriteClient`.
The point I am making is that there is no purpose for another reload here. It is just overhead that we can avoid.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513188721
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
}, {
"hash" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200",
"triggerID" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"triggerType" : "PUSH"
}, {
"hash" : "7e6238a557de32deb0131929b630e423440f8ac6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "7e6238a557de32deb0131929b630e423440f8ac6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2be8602e16700000585e0e9899891b4aed6a19b6 Azure: [CANCELED](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200)
* 7e6238a557de32deb0131929b630e423440f8ac6 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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1849386597
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
yeah, let's give it a better name.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2481840425
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 452e62f0e7efdf6bfbf1fc186cbaf99ff046494c Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709)
<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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1865047194
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
@danny0405 how about `timelineRefreshedWithinTransaction`?
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1853163566
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
How about `timelineInitializedOutsideOfTransaction` or something along those lines?
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847555196
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
There is an argument to this method called `reloadActiveTimeline` and it is true [here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/BaseCommitActionExecutor.java#L202-L203) since the table is created outside of the lock.
It is false [here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieClient.java#L239-L240). This method is called from the preCommit methods in [BaseHoodieWriteClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java#L378) and [BaseHoodieTableServiceClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java#L179). Both of these are creating the table instance which will create a new meta client right before calling the method linked in `BaseHoodieWriteClient`
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847605406
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
Thanks for the elaboration, after this change, we need to be cautious for the parameter `reloadActiveTimeline` so that the call `getCompletedInstantsDuringCurrentWriteOperation` can always check based on the latest state .
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "vinothchandar (via GitHub)" <gi...@apache.org>.
vinothchandar commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1846426925
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
+1. Within the lock, we need to reload to understand whats changed.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1865117205
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
I'm okay with it.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1866761449
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
Updated the PR
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513519448
@danny0405 I have written a new test case as well: https://github.com/apache/hudi/pull/12403
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1845912192
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
Because a concurrent writer may already complete some pending instants(from current writer's insight).
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2481763431
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 452e62f0e7efdf6bfbf1fc186cbaf99ff046494c 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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1848393340
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
We could also rename this variable so it's clear it is related to whether the timeline was refreshed within the lock
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513186874
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
}, {
"hash" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200",
"triggerID" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"triggerType" : "PUSH"
}, {
"hash" : "7e6238a557de32deb0131929b630e423440f8ac6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "7e6238a557de32deb0131929b630e423440f8ac6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 452e62f0e7efdf6bfbf1fc186cbaf99ff046494c Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709)
* 2be8602e16700000585e0e9899891b4aed6a19b6 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200)
* 7e6238a557de32deb0131929b630e423440f8ac6 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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513185217
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
}, {
"hash" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 452e62f0e7efdf6bfbf1fc186cbaf99ff046494c Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709)
* 2be8602e16700000585e0e9899891b4aed6a19b6 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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "vinothchandar (via GitHub)" <gi...@apache.org>.
vinothchandar commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1846426925
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
+1. Within the lock, we need to reload to understand whats changes.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2481764711
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 452e62f0e7efdf6bfbf1fc186cbaf99ff046494c Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709)
<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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1845820923
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
why does it always require a reload?
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1854014893
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata> resolveWriteConflictIfAny(
WriteOperationType operationType = thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy = config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation = getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(), pendingInstants);
Review Comment:
I was hoping to have some context about the transaction in the name so it is clear to future readers. I will update the javadocs as well.
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1866761614
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestSimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -412,6 +412,7 @@ public void testConcurrentWritesWithPendingInstants() throws Exception {
// make sure c3 has conflict with C1,C11,C12,C4;
HoodieCommitMetadata currentMetadata = createCommitMetadata(currentWriterInstant, "file-2");
timeline.reload();
+ metaClient.reloadActiveTimeline();
List<HoodieInstant> completedInstantsDuringCurrentWriteOperation = TransactionUtils
Review Comment:
`timeline.reload()` will return a new object, so this line isn't really doing anything since the returned instance is not used. I have removed the `timeline.reload()`
--
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] [MINOR] Remove extra timeline reload during conflict resolution [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12272:
URL: https://github.com/apache/hudi/pull/12272#issuecomment-2513366192
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=1709",
"triggerID" : "452e62f0e7efdf6bfbf1fc186cbaf99ff046494c",
"triggerType" : "PUSH"
}, {
"hash" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2200",
"triggerID" : "2be8602e16700000585e0e9899891b4aed6a19b6",
"triggerType" : "PUSH"
}, {
"hash" : "7e6238a557de32deb0131929b630e423440f8ac6",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2202",
"triggerID" : "7e6238a557de32deb0131929b630e423440f8ac6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7e6238a557de32deb0131929b630e423440f8ac6 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2202)
<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