-
Notifications
You must be signed in to change notification settings - Fork 593
High latency bundles #5826
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
High latency bundles #5826
Changes from all commits
e3c8bd6
147019f
ed1b810
cc676c7
63259b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # High-Latency Bundle Processing | ||
|
|
||
| ## Context | ||
|
|
||
| FHIR batch and transaction bundles are currently limited by `BundleConfiguration.EntryLimit`, which defaults to 500 entries. Some clients are willing to accept increased request latency in exchange for processing larger bundles. The server needs an explicit per-request opt-in while retaining a configurable upper bound. | ||
|
|
||
| ## Configuration | ||
|
|
||
| Add `BundleConfiguration.EntryLimitHighLatency` with a default value of 1,000. Add the corresponding `EntryLimitHighLatency` setting to the default web configuration. | ||
|
|
||
| The existing `EntryLimit` remains the default limit. A value of `0` continues to disable the limit for the selected processing mode. | ||
|
|
||
| ## Request Header | ||
|
|
||
| Add the request header constant: | ||
|
|
||
| `x-ms-high-latency` | ||
|
|
||
| The higher limit is selected only when the first header value parses as Boolean `true` after trimming whitespace, using case-insensitive Boolean parsing. Missing, empty, `false`, or invalid values do not opt in and use the normal entry limit. | ||
|
|
||
| ## Processing Flow | ||
|
|
||
| Add `HttpContext.IsHighLatencyEnabled()` alongside the existing header parsing extensions. `BundleHandler.FillRequestLists` determines one effective entry limit: | ||
|
|
||
| - `EntryLimitHighLatency` when `IsHighLatencyEnabled()` returns `true`. | ||
| - `EntryLimit` otherwise. | ||
|
|
||
| The handler compares the bundle entry count with the effective limit before processing entries. | ||
|
|
||
| ## Error Handling | ||
|
|
||
| Bundles above the effective limit continue to throw `BundleEntryLimitExceededException`. The existing localized error message reports the effective limit, so high-latency requests receive an error that identifies the configured high-latency maximum. | ||
|
|
||
| No new error response or status code is introduced. Invalid header values safely retain the normal limit. | ||
|
|
||
| ## Testing | ||
|
|
||
| Extend unit coverage to verify: | ||
|
|
||
| - Header parsing returns `true` only for trimmed, case-insensitive Boolean `true`. | ||
| - Missing, empty, `false`, and invalid header values return `false`. | ||
| - Requests without the header use `EntryLimit`. | ||
| - Requests with `x-ms-high-latency: true` can exceed `EntryLimit` without exceeding `EntryLimitHighLatency`. | ||
| - Requests above `EntryLimitHighLatency` throw `BundleEntryLimitExceededException`. | ||
| - The exception message contains the effective high-latency limit. | ||
|
|
||
| ## Scope | ||
|
|
||
| This change applies only to batch and transaction bundle entry-count validation. It does not change bundle execution timeouts, processing logic, orchestration, authorization, or response semantics. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,11 @@ public class BundleConfiguration | |
| { | ||
| public int EntryLimit { get; set; } = 500; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the maximum number of entries allowed when high-latency bundle processing is enabled. | ||
| /// </summary> | ||
| public int EntryLimitHighLatency { get; set; } = 1000; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we rename this property to "EntryExpandedLimit"? |
||
|
|
||
| public int MaxExecutionTimeInSeconds { get; set; } = 100; | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -586,11 +586,23 @@ private async Task ExecuteTransactionForAllRequestsAsync(Hl7.Fhir.Model.Bundle r | |
| } | ||
| } | ||
|
|
||
| private int GetEntryLimit() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe GetEntryExtendedLimit()? |
||
| { | ||
| if (_outerHttpContext.IsHighLatencyEnabled()) | ||
| { | ||
| return _bundleConfiguration.EntryLimitHighLatency; | ||
| } | ||
|
|
||
| return _bundleConfiguration.EntryLimit; | ||
| } | ||
|
|
||
| private async Task FillRequestLists(List<EntryComponent> bundleEntries, CancellationToken cancellationToken) | ||
| { | ||
| if (_bundleConfiguration.EntryLimit != default && bundleEntries.Count > _bundleConfiguration.EntryLimit) | ||
| int entryLimit = GetEntryLimit(); | ||
|
|
||
| if (entryLimit != default && bundleEntries.Count > entryLimit) | ||
| { | ||
| throw new BundleEntryLimitExceededException(string.Format(Api.Resources.BundleEntryLimitExceeded, _bundleConfiguration.EntryLimit)); | ||
| throw new BundleEntryLimitExceededException(string.Format(Api.Resources.BundleEntryLimitExceeded, entryLimit)); | ||
| } | ||
|
|
||
| int order = 0; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,6 +122,7 @@ | |
| }, | ||
| "Bundle": { | ||
| "EntryLimit": 500, | ||
| "EntryLimitHighLatency": 1000, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EntryExtendedLimit? |
||
| "SupportsBundleOrchestrator": true, | ||
| "BatchDefaultProcessingLogic": "sequential", | ||
| "TransactionDefaultProcessingLogic": "sequential" | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think high-latency is the wrong term.
I've been running tests with bundles with more than 1k conditional-operations, and they are taking 6 seconds to execute. That's not high latency based on the complexity of this type of requests.
I suggest we follow a different perspective: if customer uses a header like "x-ms-bundle-large-operations" we always handle them as parallel. That will save us time and not cause high latency. And we should probably, with the presence of this flag, assume some behaviors to optimize the execution time.
With this flag, we can also report this execution as part of a different SLI/SLO, and that should not affect our existing limits.