nsivabalan commented on code in PR #18119:
URL: https://github.com/apache/hudi/pull/18119#discussion_r2776150610
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -164,8 +164,75 @@ public Option<HoodieCommitMetadata>
resolveConflict(HoodieTable table,
return thisOperation.getCommitMetadataOption();
}
// just abort the current write if conflicts are found
- throw new HoodieWriteConflictException(new
ConcurrentModificationException("Cannot resolve conflicts for overlapping
writes between first operation = " + thisOperation
- + ", second operation = " + otherOperation));
+ throw new HoodieWriteConflictException(new
ConcurrentModificationException(buildConflictErrorMessage(thisOperation,
otherOperation)));
+ }
+
+ /**
+ * Builds a detailed error message for write conflicts based on the
operation types involved.
+ */
+ private String buildConflictErrorMessage(ConcurrentOperation thisOperation,
ConcurrentOperation otherOperation) {
+ boolean otherIsTableService =
isTableService(otherOperation.getOperationType());
+
+ String thisOpDesc = formatOperationDescription(thisOperation);
+ String otherOpDesc = formatOperationDescription(otherOperation);
+
+ // If other operation is a table service, provide specific retry guidance
+ if (otherIsTableService) {
Review Comment:
we could do this when either of them is a table service and not both.
even clustering could hit this exception when it conflicts w/ ingestion
writer.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -164,8 +164,75 @@ public Option<HoodieCommitMetadata>
resolveConflict(HoodieTable table,
return thisOperation.getCommitMetadataOption();
}
// just abort the current write if conflicts are found
- throw new HoodieWriteConflictException(new
ConcurrentModificationException("Cannot resolve conflicts for overlapping
writes between first operation = " + thisOperation
- + ", second operation = " + otherOperation));
+ throw new HoodieWriteConflictException(new
ConcurrentModificationException(buildConflictErrorMessage(thisOperation,
otherOperation)));
+ }
+
+ /**
+ * Builds a detailed error message for write conflicts based on the
operation types involved.
+ */
+ private String buildConflictErrorMessage(ConcurrentOperation thisOperation,
ConcurrentOperation otherOperation) {
+ boolean otherIsTableService =
isTableService(otherOperation.getOperationType());
+
+ String thisOpDesc = formatOperationDescription(thisOperation);
+ String otherOpDesc = formatOperationDescription(otherOperation);
+
+ // If other operation is a table service, provide specific retry guidance
+ if (otherIsTableService) {
+ String serviceType =
getTableServiceDisplayName(otherOperation.getOperationType());
+ return String.format(
+ "Cannot resolve conflicts for overlapping writes. %s is currently
running and has overlapping file groups with %s. "
+ + "Please retry the write operation after the %s completes.",
+ otherOpDesc, thisOpDesc, serviceType.toLowerCase()
+ );
+ }
+
+ // For all other cases (this is table service or both are regular
operations)
+ return String.format(
+ "Cannot resolve conflicts for overlapping writes. %s has overlapping
file groups with %s.",
+ thisOpDesc, otherOpDesc
+ );
+ }
+
+ /**
+ * Formats a description of an operation including its type, instant, and
state.
+ */
+ private String formatOperationDescription(ConcurrentOperation operation) {
+ String operationName = isTableService(operation.getOperationType())
+ ? "Table " + getTableServiceDisplayName(operation.getOperationType())
+ : operation.getOperationType().value() + " operation";
+
+ return String.format("%s (instant: %s, state: %s)",
+ operationName,
+ operation.getInstantTimestamp(),
+ operation.getInstantActionState());
+ }
+
+ /**
+ * Checks if the given operation type is a table service operation.
+ */
+ private boolean isTableService(WriteOperationType operationType) {
Review Comment:
we could move this to `WriteOperationType` class only.
we already have few methods on similar nature placed there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
