talatuyarer commented on code in PR #15308:
URL: https://github.com/apache/iceberg/pull/15308#discussion_r3139466289
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryTableOperations.java:
##########
@@ -82,48 +84,49 @@ public void doRefresh() {
// atomically
@Override
public void doCommit(TableMetadata base, TableMetadata metadata) {
- String newMetadataLocation =
- base == null && metadata.metadataFileLocation() != null
- ? metadata.metadataFileLocation()
- : writeNewMetadata(metadata, currentVersion() + 1);
- BaseMetastoreOperations.CommitStatus commitStatus =
- BaseMetastoreOperations.CommitStatus.FAILURE;
+ CommitStatus commitStatus = CommitStatus.FAILURE;
+ RetryDetector retryDetector = new RetryDetector();
+
+ String newMetadataLocation = null;
try {
- if (base == null) {
- createTable(newMetadataLocation, metadata);
+ boolean newTable = base == null;
+ newMetadataLocation = writeNewMetadataIfRequired(newTable, metadata);
+
+ if (newTable) {
+ createTable(newMetadataLocation, metadata, retryDetector);
} else {
- updateTable(newMetadataLocation, metadata);
+ updateTable(newMetadataLocation, metadata, retryDetector);
}
- commitStatus = BaseMetastoreOperations.CommitStatus.SUCCESS;
- } catch (CommitFailedException | CommitStateUnknownException e) {
+ commitStatus = CommitStatus.SUCCESS;
+ } catch (CommitFailedException e) {
throw e;
Review Comment:
I believe We must intercept the CommitFailedException which wraps the 412
Precondition Failed error when a retry has occurred, and verify if the previous
attempt succeeded.
```suggestion
if (retryDetector.retried() && newMetadataLocation != null) {
LOG.warn("Commit failed (likely 412 Etag mismatch) after retry.
Checking if previous attempt succeeded.");
commitStatus = checkCommitStatus(newMetadataLocation, metadata);
// If the check confirms the previous attempt succeeded, we are good!
if (commitStatus != CommitStatus.SUCCESS) {
// It was a real concurrent conflict with another user, rethrow
throw e;
}
} else {
// Not a retry, or no metadata location to check against. Rethrow.
throw e;
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
