Skip to content

Commit

Permalink
[Storage]Remove secondary host from retry policy, file share does not…
Browse files Browse the repository at this point in the history
… support secondary (Azure#29825)

… 


### Packages impacted by this PR


### Issues associated with this PR


### Describe the problem that is addressed by this PR


### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
  • Loading branch information
EmmaZhu authored May 29, 2024
1 parent 9fc393a commit 9b9ee0a
Show file tree
Hide file tree
Showing 10 changed files with 678 additions and 14 deletions.
2 changes: 1 addition & 1 deletion sdk/storage/storage-file-share/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "js",
"TagPrefix": "js/storage/storage-file-share",
"Tag": "js/storage/storage-file-share_152ef491fe"
"Tag": "js/storage/storage-file-share_ba58c317e8"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2061,8 +2061,7 @@ export interface StorageRetryOptions {

// @public
export class StorageRetryPolicy extends BaseRequestPolicy {
// Warning: (ae-forgotten-export) The symbol "StorageRetryOptions_2" needs to be exported by the entry point index.d.ts
constructor(nextPolicy: RequestPolicy, options: RequestPolicyOptions, retryOptions?: StorageRetryOptions_2);
constructor(nextPolicy: RequestPolicy, options: RequestPolicyOptions, retryOptions?: StorageRetryOptions);
protected attemptSendRequest(request: WebResource, secondaryHas404: boolean, attempt: number): Promise<HttpOperationResponse>;
sendRequest(request: WebResource): Promise<HttpOperationResponse>;
protected shouldRetry(isPrimaryRetry: boolean, attempt: number, response?: HttpOperationResponse, err?: RestError): boolean;
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/storage-file-share/src/Pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import {
} from "./utils/constants";
import { getCachedDefaultHttpClient } from "../../storage-blob/src/utils/cache";
import { storageBrowserPolicy } from "../../storage-blob/src/policies/StorageBrowserPolicyV2";
import { storageRetryPolicy } from "../../storage-blob/src/policies/StorageRetryPolicyV2";
import { storageRetryPolicy } from "./policies/StorageRetryPolicyV2";
import { storageSharedKeyCredentialPolicy } from "../../storage-blob/src/policies/StorageSharedKeyCredentialPolicyV2";
import { StorageBrowserPolicyFactory } from "../../storage-blob/src/StorageBrowserPolicyFactory";
import { ShareTokenIntent } from "./generatedModels";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ import {
RequestPolicyOptionsLike as RequestPolicyOptions,
RequestPolicyFactory,
} from "@azure/core-http-compat";
import {
StorageRetryPolicy,
StorageRetryPolicyType,
} from "../../storage-blob/src/policies/StorageRetryPolicy";
import { StorageRetryPolicy, StorageRetryPolicyType } from "./policies/StorageRetryPolicy";

export { StorageRetryPolicyType, StorageRetryPolicy };

Expand Down
276 changes: 276 additions & 0 deletions sdk/storage/storage-file-share/src/policies/StorageRetryPolicy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { AbortError, AbortSignalLike } from "@azure/abort-controller";

import {
RequestPolicy,
RequestPolicyOptionsLike as RequestPolicyOptions,
RequestPolicyFactory,
WebResourceLike as WebResource,
CompatResponse as HttpOperationResponse,
} from "@azure/core-http-compat";
import { BaseRequestPolicy } from "../../../storage-blob/src/policies/RequestPolicy";
import { RestError } from "@azure/core-rest-pipeline";

import { StorageRetryOptions } from "../StorageRetryPolicyFactory";
import { URLConstants } from "../utils/constants";
import { delay, setURLParameter } from "../utils/utils.common";
import { logger } from "../log";

/**
* A factory method used to generated a RetryPolicy factory.
*
* @param retryOptions -
*/
export function NewRetryPolicyFactory(retryOptions?: StorageRetryOptions): RequestPolicyFactory {
return {
create: (nextPolicy: RequestPolicy, options: RequestPolicyOptions): StorageRetryPolicy => {
return new StorageRetryPolicy(nextPolicy, options, retryOptions);
},
};
}

/**
* RetryPolicy types.
*/
export enum StorageRetryPolicyType {
/**
* Exponential retry. Retry time delay grows exponentially.
*/
EXPONENTIAL,
/**
* Linear retry. Retry time delay grows linearly.
*/
FIXED,
}

// Default values of StorageRetryOptions
const DEFAULT_RETRY_OPTIONS: StorageRetryOptions = {
maxRetryDelayInMs: 120 * 1000,
maxTries: 4,
retryDelayInMs: 4 * 1000,
retryPolicyType: StorageRetryPolicyType.EXPONENTIAL,
tryTimeoutInMs: undefined, // Use server side default timeout strategy
};

const RETRY_ABORT_ERROR = new AbortError("The operation was aborted.");

/**
* Retry policy with exponential retry and linear retry implemented.
*/
export class StorageRetryPolicy extends BaseRequestPolicy {
/**
* RetryOptions.
*/
private readonly retryOptions: StorageRetryOptions;

/**
* Creates an instance of RetryPolicy.
*
* @param nextPolicy -
* @param options -
* @param retryOptions -
*/
constructor(
nextPolicy: RequestPolicy,
options: RequestPolicyOptions,
retryOptions: StorageRetryOptions = DEFAULT_RETRY_OPTIONS,
) {
super(nextPolicy, options);

// Initialize retry options
this.retryOptions = {
retryPolicyType: retryOptions.retryPolicyType
? retryOptions.retryPolicyType
: DEFAULT_RETRY_OPTIONS.retryPolicyType,

maxTries:
retryOptions.maxTries && retryOptions.maxTries >= 1
? Math.floor(retryOptions.maxTries)
: DEFAULT_RETRY_OPTIONS.maxTries,

tryTimeoutInMs:
retryOptions.tryTimeoutInMs && retryOptions.tryTimeoutInMs >= 0
? retryOptions.tryTimeoutInMs
: DEFAULT_RETRY_OPTIONS.tryTimeoutInMs,

retryDelayInMs:
retryOptions.retryDelayInMs && retryOptions.retryDelayInMs >= 0
? Math.min(
retryOptions.retryDelayInMs,
retryOptions.maxRetryDelayInMs
? retryOptions.maxRetryDelayInMs
: DEFAULT_RETRY_OPTIONS.maxRetryDelayInMs!,
)
: DEFAULT_RETRY_OPTIONS.retryDelayInMs,

maxRetryDelayInMs:
retryOptions.maxRetryDelayInMs && retryOptions.maxRetryDelayInMs >= 0
? retryOptions.maxRetryDelayInMs
: DEFAULT_RETRY_OPTIONS.maxRetryDelayInMs,
};
}

/**
* Sends request.
*
* @param request -
*/
public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
return this.attemptSendRequest(request, false, 1);
}

/**
* Decide and perform next retry. Won't mutate request parameter.
*
* @param request -
* @param secondaryHas404 - If attempt was against the secondary & it returned a StatusNotFound (404), then
* the resource was not found. This may be due to replication delay. So, in this
* case, we'll never try the secondary again for this operation.
* @param attempt - How many retries has been attempted to performed, starting from 1, which includes
* the attempt will be performed by this method call.
*/
protected async attemptSendRequest(
request: WebResource,
secondaryHas404: boolean,
attempt: number,
): Promise<HttpOperationResponse> {
const newRequest: WebResource = request.clone();

const isPrimaryRetry = true;

// Set the server-side timeout query parameter "timeout=[seconds]"
if (this.retryOptions.tryTimeoutInMs) {
newRequest.url = setURLParameter(
newRequest.url,
URLConstants.Parameters.TIMEOUT,
Math.floor(this.retryOptions.tryTimeoutInMs! / 1000).toString(),
);
}

let response: HttpOperationResponse | undefined;
try {
logger.info(`RetryPolicy: =====> Try=${attempt} ${isPrimaryRetry ? "Primary" : "Secondary"}`);
response = await this._nextPolicy.sendRequest(newRequest);
if (!this.shouldRetry(isPrimaryRetry, attempt, response)) {
return response;
}

secondaryHas404 = secondaryHas404 || (!isPrimaryRetry && response.status === 404);
} catch (err: any) {
logger.error(`RetryPolicy: Caught error, message: ${err.message}, code: ${err.code}`);
if (!this.shouldRetry(isPrimaryRetry, attempt, response, err)) {
throw err;
}
}

await this.delay(isPrimaryRetry, attempt, request.abortSignal);
return this.attemptSendRequest(request, secondaryHas404, ++attempt);
}

/**
* Decide whether to retry according to last HTTP response and retry counters.
*
* @param isPrimaryRetry -
* @param attempt -
* @param response -
* @param err -
*/
protected shouldRetry(
isPrimaryRetry: boolean,
attempt: number,
response?: HttpOperationResponse,
err?: RestError,
): boolean {
if (attempt >= this.retryOptions.maxTries!) {
logger.info(
`RetryPolicy: Attempt(s) ${attempt} >= maxTries ${this.retryOptions
.maxTries!}, no further try.`,
);
return false;
}

// Handle network failures, you may need to customize the list when you implement
// your own http client
const retriableErrors = [
"ETIMEDOUT",
"ESOCKETTIMEDOUT",
"ECONNREFUSED",
"ECONNRESET",
"ENOENT",
"ENOTFOUND",
"TIMEOUT",
"EPIPE",
"REQUEST_SEND_ERROR", // For default xhr based http client provided in ms-rest-js
];
if (err) {
for (const retriableError of retriableErrors) {
if (
err.name.toUpperCase().includes(retriableError) ||
err.message.toUpperCase().includes(retriableError) ||
(err.code && err.code.toString().toUpperCase() === retriableError)
) {
logger.info(`RetryPolicy: Network error ${retriableError} found, will retry.`);
return true;
}
}
}

// If attempt was against the secondary & it returned a StatusNotFound (404), then
// the resource was not found. This may be due to replication delay. So, in this
// case, we'll never try the secondary again for this operation.
if (response || err) {
const statusCode = response ? response.status : err ? err.statusCode : 0;
if (!isPrimaryRetry && statusCode === 404) {
logger.info(`RetryPolicy: Secondary access with 404, will retry.`);
return true;
}

// Server internal error or server timeout
if (statusCode === 503 || statusCode === 500) {
logger.info(`RetryPolicy: Will retry for status code ${statusCode}.`);
return true;
}
}

if (err?.code === "PARSE_ERROR" && err?.message.startsWith(`Error "Error: Unclosed root tag`)) {
logger.info(
"RetryPolicy: Incomplete XML response likely due to service timeout, will retry.",
);
return true;
}

return false;
}

/**
* Delay a calculated time between retries.
*
* @param isPrimaryRetry -
* @param attempt -
* @param abortSignal -
*/
private async delay(isPrimaryRetry: boolean, attempt: number, abortSignal?: AbortSignalLike) {
let delayTimeInMs: number = 0;

if (isPrimaryRetry) {
switch (this.retryOptions.retryPolicyType) {
case StorageRetryPolicyType.EXPONENTIAL:
delayTimeInMs = Math.min(
(Math.pow(2, attempt - 1) - 1) * this.retryOptions.retryDelayInMs!,
this.retryOptions.maxRetryDelayInMs!,
);
break;
case StorageRetryPolicyType.FIXED:
delayTimeInMs = this.retryOptions.retryDelayInMs!;
break;
}
} else {
delayTimeInMs = Math.random() * 1000;
}

logger.info(`RetryPolicy: Delay for ${delayTimeInMs}ms`);
return delay(delayTimeInMs, abortSignal, RETRY_ABORT_ERROR);
}
}
Loading

0 comments on commit 9b9ee0a

Please sign in to comment.