Re: [PR] [HUDI-7710] Remove compaction.inflight from conflict resolution [hudi]
danny0405 commented on code in PR #11148: URL: https://github.com/apache/hudi/pull/11148#discussion_r1590186936 ## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java: ## @@ -68,6 +69,7 @@ public Stream getCandidateInstants(HoodieTableMetaClient metaClie .getTimelineOfActions(CollectionUtils.createSet(REPLACE_COMMIT_ACTION, COMPACTION_ACTION)) .findInstantsAfter(currentInstant.getTimestamp()) .filterInflightsAndRequested() +.filter(i -> (!i.getAction().equals(COMPACTION_ACTION)) || i.getState().equals(REQUESTED)) .getInstantsAsStream(); Review Comment: I guess if the compaction does not really execute before, there is no need to resolve the conflicts, because the log files would slice based on their specific completion time. If there is no confclits for the same file group from multiple writers, then we are good. @linliu-code , we can add some test cases to illustrate this. -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove compaction.inflight from conflict resolution [hudi]
yihua commented on code in PR #11148: URL: https://github.com/apache/hudi/pull/11148#discussion_r1590072865 ## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java: ## @@ -68,6 +69,7 @@ public Stream getCandidateInstants(HoodieTableMetaClient metaClie .getTimelineOfActions(CollectionUtils.createSet(REPLACE_COMMIT_ACTION, COMPACTION_ACTION)) .findInstantsAfter(currentInstant.getTimestamp()) .filterInflightsAndRequested() +.filter(i -> (!i.getAction().equals(COMPACTION_ACTION)) || i.getState().equals(REQUESTED)) .getInstantsAsStream(); Review Comment: @linliu-code We still need to check the compaction for conflict correct? So instead of filtering out `compaction.inflight`, we should convert `instant.compaction.inflight` to `instant.compaction.request` for checking? Can you write a test case for this? -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
danny0405 merged PR #11148: URL: https://github.com/apache/hudi/pull/11148 -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
danny0405 commented on code in PR #11148: URL: https://github.com/apache/hudi/pull/11148#discussion_r1589927822 ## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java: ## @@ -68,6 +69,7 @@ public Stream getCandidateInstants(HoodieTableMetaClient metaClie .getTimelineOfActions(CollectionUtils.createSet(REPLACE_COMMIT_ACTION, COMPACTION_ACTION)) .findInstantsAfter(currentInstant.getTimestamp()) .filterInflightsAndRequested() +.filter(i -> (!i.getAction().equals(COMPACTION_ACTION)) || i.getState().equals(REQUESTED)) .getInstantsAsStream(); Review Comment: Looks reasonable. -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
hudi-bot commented on PR #11148: URL: https://github.com/apache/hudi/pull/11148#issuecomment-2094072154 ## CI report: * 82ace4ec10ccae4108bed6f67674390f905eee7f Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=23654) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
hudi-bot commented on PR #11148: URL: https://github.com/apache/hudi/pull/11148#issuecomment-2094024555 ## CI report: * 82ace4ec10ccae4108bed6f67674390f905eee7f Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=23654) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
hudi-bot commented on PR #11148: URL: https://github.com/apache/hudi/pull/11148#issuecomment-2094020901 ## CI report: * 82ace4ec10ccae4108bed6f67674390f905eee7f UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [HUDI-7710] Remove `compaction.inflight` from conflict resolution [hudi]
linliu-code opened a new pull request, #11148: URL: https://github.com/apache/hudi/pull/11148 ### Change Logs We filter it from `SimpleConcurrentFileWritesConflictResolutionStrategy` class. ### Impact Fix a buggy process. ### Risk level (write none, low medium or high below) None. ### 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-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
