nastra commented on code in PR #8857:
URL: https://github.com/apache/iceberg/pull/8857#discussion_r1392591040
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -540,4 +585,64 @@ public void close() {
api.close();
}
}
+
+ private void commitRetry(String message, Operation... ops)
+ throws BaseNessieClientServerException {
+ commitRetry(message, false, builder ->
builder.operations(Arrays.asList(ops)));
+ }
+
+ private void commitRetry(String message, boolean retryConflicts,
CommitEnhancer commitEnhancer)
+ throws BaseNessieClientServerException {
+ // Retry all errors except for NessieNotFoundException and also
NessieConflictException, unless
+ // retryConflicts is set to true.
+ Predicate<Exception> shouldRetry =
+ e ->
+ !(e instanceof NessieNotFoundException)
+ && (!(e instanceof NessieConflictException) || retryConflicts);
+ Tasks.range(1)
+ .retry(5)
+ .shouldRetryTest(shouldRetry)
+ .throwFailureWhenFinished()
+ .onFailure((o, exception) -> refresh()) // FIXME is this necessary?
Review Comment:
I was rather pointing out that we should remove the FIXME itself here. I'm
ok if you want to tackle the underlying problem separately, but the FIXME just
raised the question whether the refresh is necessary or not (not that you want
to tackle it separately).
For some historical context, the reason why a refresh on failure was
introduced is described in https://github.com/apache/iceberg/pull/2519, so you
might want to check whether that still applies or not.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
