smiroslav commented on code in PR #1748:
URL: https://github.com/apache/jackrabbit-oak/pull/1748#discussion_r1852120644


##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/AzureRequestOptions.java:
##########
@@ -45,52 +43,43 @@ public class AzureRequestOptions {
     private AzureRequestOptions() {
     }
 
-    /**
-     * Apply default request options to the blobRequestOptions if they are not 
already set.
-     * @param blobRequestOptions
-     */
-    public static void applyDefaultRequestOptions(BlobRequestOptions 
blobRequestOptions) {
-        if (blobRequestOptions.getRetryPolicyFactory() == null) {
-            int retryAttempts = Integer.getInteger(RETRY_ATTEMPTS_PROP, 
DEFAULT_RETRY_ATTEMPTS);
-            if (retryAttempts > 0) {
-                Integer retryBackoffSeconds = 
Integer.getInteger(RETRY_BACKOFF_PROP, DEFAULT_RETRY_BACKOFF_SECONDS);
-                blobRequestOptions.setRetryPolicyFactory(new 
RetryLinearRetry((int) TimeUnit.SECONDS.toMillis(retryBackoffSeconds), 
retryAttempts));
-            }
-        }
-        if (blobRequestOptions.getMaximumExecutionTimeInMs() == null) {
-            int timeoutExecution = Integer.getInteger(TIMEOUT_EXECUTION_PROP, 
DEFAULT_TIMEOUT_EXECUTION);
-            if (timeoutExecution > 0) {
-                blobRequestOptions.setMaximumExecutionTimeInMs((int) 
TimeUnit.SECONDS.toMillis(timeoutExecution));
-            }
-        }
-        if (blobRequestOptions.getTimeoutIntervalInMs() == null) {
-            int timeoutInterval = Integer.getInteger(TIMEOUT_INTERVAL_PROP, 
DEFAULT_TIMEOUT_INTERVAL);
-            if (timeoutInterval > 0) {
-                blobRequestOptions.setTimeoutIntervalInMs((int) 
TimeUnit.SECONDS.toMillis(timeoutInterval));
-            }
-        }
+
+    public static RequestRetryOptions getRetryOptionsDefault() {
+        return getRetryOptionsDefault(null);
+    }
+
+    public static RequestRetryOptions getRetryOptionsDefault(String 
secondaryHost) {

Review Comment:
   From the API `RequestRetryOptions` constructor has the following arguments: 
   * `retryPolicyType ` - this one should be `RetryPolicyType.FIXED` 
:white_check_mark:
   * `maxTries` - this should come from RETRY_ATTEMPTS_PROP 
("segment.azure.retry.attempts") :white_check_mark:
   * `tryTimeoutInSeconds` - this would be max timeout for one retry, 
initialised with TIMEOUT_INTERVAL_PROP ("segment.timeout.interval") :x: 
currently in the code it takes value from TIMEOUT_EXECUTION_PROP
   * `retryDelayInMs` - should come from RETRY_BACKOFF_PROP 
("segment.azure.retry.backoff")  :white_check_mark:
   * `maxRetryDelayInMs` - probably the same value as for `retryDelayInMs` can 
be used
   
   Unused system properties should be deleted from `AzureRequestOptions`
   
   This link explains retry options in detail
   
https://learn.microsoft.com/en-us/azure/storage/blobs/storage-retry-policy-java
   
    



-- 
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]

Reply via email to