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 2025/05/18 21:46:48 UTC
[PR] [HUDI-9388] Add compaction and clustering plan conflict detection logic [hudi]
the-other-tim-brown opened a new pull request, #13316:
URL: https://github.com/apache/hudi/pull/13316
### Change Logs
- Adds a helper class that can be used for detecting conflicts with table service plans and new actions on the timeline. This will allow us to move the service planning outside of the lock which will help minimize the time the lock is held and help with throughput of concurrent writers.
### Impact
- This sets us up for more improvements that will help with throughput of concurrent writers and async table services
### Risk level (write none, low medium or high below)
Low, this PR uses existing functionality for conflict detection and does not wire any of this into the table service planning at this point in time. That will come in a future PR
### 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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103655628
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
> a. completed ingestion commit. and compaction plan that just got generated is going through conflict resolution.
if there are overlapping file groups, we are failing here. bcoz, the cur plan generated would not have included log files produced by an inglith ingestion commit.
The v8 can never include inflight log files, actually our fs view never expose any uncommitted log files, the planner would filter out thouse log files finished after it's requested compaction time.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101383736
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/ConcurrentOperation.java:
##########
@@ -84,6 +86,38 @@ public ConcurrentOperation(HoodieInstant instant, HoodieCommitMetadata commitMet
init(instant);
}
+ /**
+ * Construct a ConcurrentOperation object for a compaction plan that is not yet persisted to the timeline.
+ * This is used to detect potential conflicts with other operations.
+ * @param compactionPlan the potential compaction plan
+ * @param requestedInstantTime the requested instant time for the compaction
+ */
+ public ConcurrentOperation(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ this.commitMetadataOption = Option.empty();
+ this.metadataWrapper = null;
Review Comment:
Let's move the `metadataWrapper` to be a local variable used only for `#init` purposes.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2889230291
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ad48bf973a8700435d424f19c9216abcf9c5d52f Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535)
<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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2102669981
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
I have updated the description. The main issue stems from the fact that the plans contain the paths that are meant to be included in the operation. There is a potential to miss files as a result. This only happens when there is a delta commit that completes between starting the plan and then trying to persist the plan which should be a relatively small window. The planners are also faster now that they have incremental support similar so this is not a lot of wasted work.
The code also has had javadocs for this content https://github.com/apache/hudi/pull/13316/files#diff-193af01d6712d3cc32a155903811e4562a966bbadc8dff1b4a97691744267c6cR54 - feel free to comment inline there 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] [HUDI-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103564359
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Added a new PR for this https://github.com/apache/hudi/pull/13347
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2899415304
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ad48bf973a8700435d424f19c9216abcf9c5d52f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535)
* dfb9b16f95d533c5773d568089f3dfce719fed1b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627)
<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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103679208
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Here is the scenario that causes the issue:
T1: Delta commit (DC1) starts
T2: Compaction planner starts and ignores log files from DC1 since DC1 is not complete
T3: DC1 completes
T4: Requested timestamp is generated and larger than completion time of DC1
T5: Compaction plan is persisted (compaction plans contain the exact list of files to compact)
T6: Compaction executes and misses the log files of DC1
Those log files from DC1 will not be part of the new file group created by compaction since the completion time of DC1 is before the requested time of the compaction plan.
In the future if we want the compaction plan to strictly list the groups to compact instead of the specific files, then we can potentially handle this case more gracefully.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101282629
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
+ } catch (IOException ex) {
+ throw new HoodieIOException("Unable to parse instant details for conflict detection", ex);
+ }
+ });
+ }
+
+ private Stream<HoodieInstant> generateCandidateInstants() {
+ Stream<HoodieInstant> commitsCompletedDuringPlanning = metaClient
Review Comment:
There was a test but I had not refreshed the timeline so it was not working as expected
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2889229623
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ad48bf973a8700435d424f19c9216abcf9c5d52f 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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103559050
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/ConcurrentOperation.java:
##########
@@ -84,6 +86,38 @@ public ConcurrentOperation(HoodieInstant instant, HoodieCommitMetadata commitMet
init(instant);
}
+ /**
+ * Construct a ConcurrentOperation object for a compaction plan that is not yet persisted to the timeline.
+ * This is used to detect potential conflicts with other operations.
+ * @param compactionPlan the potential compaction plan
+ * @param requestedInstantTime the requested instant time for the compaction
+ */
+ public ConcurrentOperation(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ this.commitMetadataOption = Option.empty();
+ this.metadataWrapper = null;
Review Comment:
Makes sense, 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] [HUDI-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2102683609
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
I think the conflict detection on write needs to be updated for the v8 tables
https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java#L59
This looks like we may fail the write if there is a compaction planned after the delta commit starts but this should be fine with completion time ordering. I will look into this today in a separate PR to make sure we have some testing around 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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103668092
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Right, but the idea here is that the planner will not require a lock to start the plan in the future so it does not block while creating the plan. Then if there is a plan, the following steps will happen:
1. Transaction is started (lock is taken)
2. Check for conflicts and exit early if any detected
3. Instant time is generated for the requested instant
4. Persist the plan to the timeline
5. End transaction (lock is released)
This follows the general pattern we're trying to adopt where the instant time is generated and persisted within the same transaction.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2899412142
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ad48bf973a8700435d424f19c9216abcf9c5d52f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535)
* dfb9b16f95d533c5773d568089f3dfce719fed1b 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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2889258993
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ad48bf973a8700435d424f19c9216abcf9c5d52f Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535)
<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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103674424
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
> That gap between the plan being generated and the lock being acquired is where can have issues.
sill okay for compaction I think, no matter whether there are new delta commits negerated during this time gap.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103192742
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
We need to revisit conflict resolution strategy so that it honors completion time ordering and make sure compaction scheduling does not block deltacommits based on the completion-time-based file slicing in table version 8.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101398334
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Can the `conflictResolutionStrategy` handle the case: even if there is a concurrent modification of file group from a commit/delta_commit and the compaction plan, the plan is still valid because we have the completion time based file slicing and we filter the files that should not be included.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101431783
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
but here is my understanding.
we need to distinguish if its an inflight ingestion commit or completed.
a. completed ingestion commit. and compaction plan that just got generated is going through conflict resolution.
if there are overlapping file groups, we are failing here. bcoz, the cur plan generated would not have included log files produced by an inglith ingestion commit.
b. inflight ingestion commit. and compaction plan that just got generated is going through conflict resolution.
here we let the compaction plan go ahead. since compaction plan conflict resolution should be happening within a lock and once the resolution succeeds, compaction.requested will be added to the timeline. and hence the ingestion commit's completion time will definitely be > the compaction instant time.
in this case, lets also see how does conflict resolution of entire write will happen for ingestion commit (I am not talking about TableServicePlanConflictDetection).
- Guess we might need a fix here. currently(as per master), if the ingestion commits's start time is < compaction instant time, we fail the resolution step. if its >, we let is pass.
we might need to make changes to this once we have this patch landed. essentially, ingestion commits can never conflict w/ a compaction commit during the final conflict resolution.
But I will let Tim also go over this and see if my understanding is right.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103783073
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
For compaction, things should be different, we can use T2 to schedule the plan and persisted the plan with T2 if there is no conflict, like I mentioned, the requested time of compaction is just a barrier, it's sequence does not affect the correctness.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103775699
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
The commit time is going to be generated in the same transaction that persists that instant.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "vinothchandar (via GitHub)" <gi...@apache.org>.
vinothchandar commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2903295893
@danny0405 please summarize all your pending concerns here.
Without much context on the broader discussion, I want to ensure we make things simpler first.. and avoid premature perf optimizations..
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101399933
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
This is very critical support, otherwise we could have many unnecessary job failover because of the conflicts.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103668092
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Right, but the idea here is that the planner will not require a lock to start the plan in the future so it does not block while creating the plan. Then if there is a plan, the following steps will happen:
1. Transaction is started (lock is taken)
2. Check for conflicts and exit early if any detected
3. Instant time is generated for the requested instant
4. Persist the plan to the timeline
5. End transaction (lock is released)
This follows the general pattern we're trying to adopt where the instant time is generated and persisted within the same transaction. That gap between the plan being generated and the lock being acquired is where can have 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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2902847216
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
}, {
"hash" : "6ed2ad506732def4b51169667a064847bca9539b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6ed2ad506732def4b51169667a064847bca9539b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dfb9b16f95d533c5773d568089f3dfce719fed1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627)
* 6ed2ad506732def4b51169667a064847bca9539b 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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2905154172
Closing this, will move the planning and instant generation in the lock to avoid the need for such conflict detection. We can focus on making planning more efficient with the recent improvements in incremental planning and metadata table performance.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2103769621
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
Why compaction uses T4 instead of T2 as the requested instant time, confused.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101421409
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
+ } catch (IOException ex) {
+ throw new HoodieIOException("Unable to parse instant details for conflict detection", ex);
+ }
+ });
+ }
+
+ private Stream<HoodieInstant> generateCandidateInstants() {
+ Stream<HoodieInstant> commitsCompletedDuringPlanning = metaClient
Review Comment:
yeah, I also did see that test and was puzzling.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101424386
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
Review Comment:
sounds fair. have asked Tim to add more details to PR description.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2903415341
> @danny0405 please summarize all your pending concerns here.
>
> Without much context on the broader discussion, I want to ensure we make things simpler first.. and avoid premature perf optimizations..
Sorry, I don't think I will give design decision everywhere, I'm exhausted with all the PR reviewing of the recent core changes, will just put strong opinions if I see some critical issues here. Just don't want to see we output a fragile release in general.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown closed pull request #13316: [HUDI-9388] Add compaction and clustering plan conflict detection logic
URL: https://github.com/apache/hudi/pull/13316
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2902962106
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
}, {
"hash" : "6ed2ad506732def4b51169667a064847bca9539b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5660",
"triggerID" : "6ed2ad506732def4b51169667a064847bca9539b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6ed2ad506732def4b51169667a064847bca9539b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5660)
<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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2902849441
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
}, {
"hash" : "6ed2ad506732def4b51169667a064847bca9539b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5660",
"triggerID" : "6ed2ad506732def4b51169667a064847bca9539b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dfb9b16f95d533c5773d568089f3dfce719fed1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627)
* 6ed2ad506732def4b51169667a064847bca9539b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5660)
<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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101255379
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
+ } catch (IOException ex) {
+ throw new HoodieIOException("Unable to parse instant details for conflict detection", ex);
+ }
+ });
+ }
+
+ private Stream<HoodieInstant> generateCandidateInstants() {
+ Stream<HoodieInstant> commitsCompletedDuringPlanning = metaClient
Review Comment:
this is not necessarily completed right? why name it so?
this is referring to pendingOrCompletedWriteCommits.
or did you miss completed() filter in L 93, 94.
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #13316:
URL: https://github.com/apache/hudi/pull/13316#discussion_r2101262423
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/TableServicePlanConflictDetection.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction;
+
+import org.apache.hudi.avro.model.HoodieClusteringPlan;
+import org.apache.hudi.avro.model.HoodieCompactionPlan;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+/**
+ * Handles conflict detection for the plans of various table services.
+ * Cleaning and archiving are not included in this class as they do not require conflict detection.
+ */
+public class TableServicePlanConflictDetection {
+ private final ConflictResolutionStrategy conflictResolutionStrategy;
+ private final HoodieTableMetaClient metaClient;
+ private final String lastKnownCompletionTime;
+
+ /**
+ * Constructors a new instance to detect conflicts for a table service plan.
+ * Note that this should be used within a transaction to prevent race conditions. The meta client used must be constructed or reloaded within the transaction.
+ * @param conflictResolutionStrategy The strategy to use for conflict resolution for the writer.
+ * @param metaClient The meta client for the table. This must be constructed or reloaded during the transaction.
+ * @param lastKnownCompletionTime The last completion time of an instant on the timeline when the planner started.
+ */
+ public TableServicePlanConflictDetection(ConflictResolutionStrategy conflictResolutionStrategy, HoodieTableMetaClient metaClient, String lastKnownCompletionTime) {
+ this.conflictResolutionStrategy = conflictResolutionStrategy;
+ this.metaClient = metaClient;
+ this.lastKnownCompletionTime = lastKnownCompletionTime;
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The compaction requires a check for conflicts with any commits for ingesting new data since the compaction plan is expected to contain all the log files for the slice it is compacting.
+ * It also requires checks for new plans for clustering or potentially another compaction on the timeline to avoid multiple services operating on the same file groups.
+ * @param compactionPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieCompactionPlan compactionPlan, String requestedInstantTime) {
+ ConcurrentOperation compactionOperation = new ConcurrentOperation(compactionPlan, requestedInstantTime);
+ return hasConflict(compactionOperation);
+ }
+
+ /**
+ * Checks if there are any potential conflicts with actions that have happened since the planner start instant.
+ * The clustering requires a check for conflicts with any commits for ingesting new data since the clustering plan will also contain log files,
+ * and all the log files for the slice it is clustering must be in the plan.
+ * It also requires checks for new plans for compaction or potentially another clustering on the timeline to avoid multiple services operating on the same file groups.
+ * @param clusteringPlan the requested plan
+ * @param requestedInstantTime the requested instant time that will be used when persisting the plan to the timeline
+ * @return true if there is a conflict with the requested plan, false otherwise
+ */
+ public boolean hasConflict(HoodieClusteringPlan clusteringPlan, String requestedInstantTime) {
+ ConcurrentOperation clusteringOperation = new ConcurrentOperation(clusteringPlan, requestedInstantTime);
+ return hasConflict(clusteringOperation);
+ }
+
+ private boolean hasConflict(ConcurrentOperation plannedOperation) {
+ return generateCandidateInstants().anyMatch(instant -> {
+ try {
+ ConcurrentOperation otherOperation = new ConcurrentOperation(instant, metaClient);
+ return conflictResolutionStrategy.hasConflict(plannedOperation, otherOperation);
+ } catch (IOException ex) {
+ throw new HoodieIOException("Unable to parse instant details for conflict detection", ex);
+ }
+ });
+ }
+
+ private Stream<HoodieInstant> generateCandidateInstants() {
+ Stream<HoodieInstant> commitsCompletedDuringPlanning = metaClient
Review Comment:
I meant to add completed, will update and add a test
--
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-9388] Add compaction and clustering plan conflict detection logic [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13316:
URL: https://github.com/apache/hudi/pull/13316#issuecomment-2899528793
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5535",
"triggerID" : "ad48bf973a8700435d424f19c9216abcf9c5d52f",
"triggerType" : "PUSH"
}, {
"hash" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627",
"triggerID" : "dfb9b16f95d533c5773d568089f3dfce719fed1b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dfb9b16f95d533c5773d568089f3dfce719fed1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=5627)
<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