maytasm commented on PR #14776: URL: https://github.com/apache/druid/pull/14776#issuecomment-1670499066
> > > > (I am not 100% sure but) this may not work because we are using deleteObjects, which returns MultiObjectDeleteException. MultiObjectDeleteException error code is null (https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/model/MultiObjectDeleteException.html#getErrorCode--) which may not cause us to retry. > > > > Basically the "retry-able" error is actually inside the MultiObjectDeleteException list of Errors but MultiObjectDeleteException itself may not be retry-able. I took a look at AWSClientUtil.isClientExceptionRecoverable and it seems like it would depend on ` public static boolean isRetryableServiceException(AmazonServiceException exception) { return RETRYABLE_STATUS_CODES.contains(exception.getStatusCode()) || RETRYABLE_ERROR_CODES.contains(exception.getErrorCode()) || reasonPhraseMatchesErrorCode(exception, RETRYABLE_ERROR_CODES); }` Since MultiObjectDeleteException getErrorCode always returns null (as indicated by the doc), the second and third checks would always be false. I am not about the status code but it might be 200. > > > > > > > > > darn I think you're right. Hmm. In this case, should we just always retry up to 3 times regardless of the error? > > > > > > Maybe you can iterate the errors in multideletefailure object and either > > ``` > > * (smarter way?) filter out all non retry-able before retrying (only retry the paths that has retry-able error) > > > > * (super simple way) just retry if any of the exception is retry-able (so you don’t have the modify the request) — this is still a little better than always retrying blindly > > ``` > > @maytasm , There doesn't seem to be any public method that aws sdk exposes to check whether a particular errorCode is recoverable, only whether an exception is recoverable. This exception type as you stated, exposes a list of Errors, which do not include the particular exception type, only the code, message, and a few other things. There are several other ways for this retry mechanism to view the exception as retryable, besides the errorCode, as you stated, such as the statusCode, etc. Let me know what you think. You are right. Seems like the Error in MultiObjectDeleteException are not that useful in term of determining if we can retry or not. I have another idea. If a batch delete (deleteObjects) fails, we get the individual failed keys and use the single delete (deleteObject) with the RetryUtils.retry block for each single delete call. The single delete can work with the RetryUtils.retry since we can determine if it is retry-able or not from the Exception thrown. The other option is to retry on MultiObjectDeleteException regardless of checking the individual Error contained in the error list. @jasonk000 @TSFenwick Any other suggestions? -- 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]
