Ppaul/sdk pruning failover retries#60200
Open
PallabPaul wants to merge 12 commits into
Open
Conversation
…collection fallback
PallabPaul
requested review from
a team,
andpiccione,
ivarprudnikov,
msftsettiy,
musabbir,
ryazhang-microsoft and
weirenlai
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Confidential Ledger client’s resiliency behavior by adding failover ledger routing, optional “archived/pruned collection” fallback for GetCurrentLedgerEntry, improved polling tolerance in PostLedgerEntryOperation, and new tests validating these behaviors. It also bumps the package version and updates the changelog for the new features.
Changes:
- Added a failover discovery/execution service and integrated it into
GetLedgerEntry*/GetCurrentLedgerEntry*. - Added optional archived-collection fallback and “Loading” polling behavior driven by client retry settings.
- Updated
PostLedgerEntryOperationpolling to tolerate transient406and bounded consecutive404s; added new resiliency tests and updated version/changelog.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/tests/ConfidentialLedgerPruningTests.cs | Adds unit tests covering archived fallback, loading polling, and 406/404 tolerance. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/PostLedgerEntryOperation.cs | Adds logic to treat 406 as pending and tolerate a bounded number of consecutive 404s while polling. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/Generated/ConfidentialLedgerClient.cs | Adds failover routing and “Loading” polling to protocol methods, plus helper request overloads. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/ConfidentialLedgerFailoverService.cs | Introduces failover endpoint discovery and execution logic. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/ConfidentialLedgerClientOptions.cs | Adds EnableArchivedCollectionFallback option. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/ConfidentialLedgerClient.cs | Wires in failover service, archived fallback, and “Loading” detection/formatting helpers. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/src/Azure.Security.ConfidentialLedger.csproj | Bumps package version to 1.5.0-beta.1. |
| sdk/confidentialledger/Azure.Security.ConfidentialLedger/CHANGELOG.md | Documents new features and bug fixes for 1.5.0-beta.1. |
Comment on lines
+277
to
+287
| Response failoverCurrent = await _failoverService.ExecuteOnFailoversAsync( | ||
| _ledgerEndpoint, | ||
| async (endpoint) => | ||
| { | ||
| using HttpMessage message = CreateGetCurrentLedgerEntryRequest(endpoint, collectionId, context); | ||
| return await _pipeline.ProcessMessageAsync(message, context).ConfigureAwait(false); | ||
| }, | ||
| nameof(GetCurrentLedgerEntryAsync), | ||
| collectionId, | ||
| context?.CancellationToken ?? default).ConfigureAwait(false); | ||
| return FormatLedgerEntry(failoverCurrent); |
Comment on lines
+297
to
+305
| if (!IsLoadingResponse(response) || attempt >= _maxLoadingRetries) | ||
| { | ||
| return response; | ||
| } | ||
|
|
||
| if (_loadingPollDelay > TimeSpan.Zero) | ||
| { | ||
| await Task.Delay(_loadingPollDelay, cancellationToken).ConfigureAwait(false); | ||
| } |
Comment on lines
+362
to
+373
| Response failoverCurrent = _failoverService.ExecuteOnFailovers( | ||
| _ledgerEndpoint, | ||
| (endpoint) => | ||
| { | ||
| using HttpMessage message = CreateGetCurrentLedgerEntryRequest(endpoint, collectionId, context); | ||
| return _pipeline.ProcessMessage(message, context); | ||
| }, | ||
| nameof(GetCurrentLedgerEntry), | ||
| collectionId, | ||
| context?.CancellationToken ?? default); | ||
| return FormatLedgerEntry(failoverCurrent); | ||
| } |
Comment on lines
+382
to
+391
| if (!IsLoadingResponse(response) || attempt >= _maxLoadingRetries) | ||
| { | ||
| return response; | ||
| } | ||
|
|
||
| if (_loadingPollDelay > TimeSpan.Zero) | ||
| { | ||
| cancellationToken.WaitHandle.WaitOne(_loadingPollDelay); | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| } |
Comment on lines
+16
to
+32
| private readonly HttpPipeline _pipeline; | ||
| private readonly ClientDiagnostics _clientDiagnostics; | ||
|
|
||
| // The identity service used to discover failover ledgers. Honors a custom | ||
| // ConfidentialLedgerClientOptions.CertificateEndpoint when one is configured, falling back to | ||
| // the default identity service endpoint otherwise. | ||
| private readonly Uri _identityServiceEndpoint; | ||
|
|
||
| private static ResponseClassifier _responseClassifier200; | ||
| private static ResponseClassifier ResponseClassifier200 => _responseClassifier200 ??= new StatusCodeClassifier(stackalloc ushort[] { 200 }); | ||
|
|
||
| public ConfidentialLedgerFailoverService(HttpPipeline pipeline, ClientDiagnostics clientDiagnostics, Uri identityServiceEndpoint = null) | ||
| { | ||
| _pipeline = pipeline ?? throw new ArgumentNullException(nameof(pipeline)); | ||
| _clientDiagnostics = clientDiagnostics ?? throw new ArgumentNullException(nameof(clientDiagnostics)); | ||
| _identityServiceEndpoint = identityServiceEndpoint ?? new Uri(ConfidentialLedgerClient.Default_Certificate_Endpoint); | ||
| } |
Comment on lines
+41
to
+45
| if (string.IsNullOrEmpty(collectionIdGate)) | ||
| { | ||
| return operationAsync(primaryEndpoint); // collection gating: no failover | ||
| } | ||
| return ExecuteOnFailoversAsync(primaryEndpoint, operationAsync, operationName, cancellationToken); |
Comment on lines
+55
to
+59
| if (string.IsNullOrEmpty(collectionIdGate)) | ||
| { | ||
| return operationSync(primaryEndpoint); | ||
| } | ||
| return ExecuteOnFailovers(primaryEndpoint, operationSync, operationName, cancellationToken); |
Comment on lines
+72
to
+74
| using HttpMessage message = CreateFailoverRequest(failoverUrl); | ||
| Response response = await _pipeline.ProcessMessageAsync(message, new RequestContext()).ConfigureAwait(false); | ||
| return ParseFailoverEndpoints(primaryEndpoint, response); |
Comment on lines
+94
to
+96
| using HttpMessage message = CreateFailoverRequest(failoverUrl); | ||
| Response response = _pipeline.ProcessMessage(message, new RequestContext()); | ||
| return ParseFailoverEndpoints(primaryEndpoint, response); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.