-
Notifications
You must be signed in to change notification settings - Fork 50
Large payload azure blob externalization support #468
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?
Conversation
…/durabletask-dotnet into wangbill/large-payload
…t/durabletask-dotnet into wangbill/largepayloadv2
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.
Nothing opposing the core functionality, but my main concern is with the fact that this has to be setup on the client and the worker. I'm worried that users could provide different configurations and break their payload flow.
Do you think it'd be possible to instead make the client/worker extensions depend directly on inheriting an interface/abstract class that we could instantiate instead? By having the user instantiate the class itself and then pass that around, we make chances for misconfiguration go down.
src/Extensions/AzureBlobPayloads/Interceptors/BasePayloadInterceptor.cs
Outdated
Show resolved
Hide resolved
src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs
Outdated
Show resolved
Hide resolved
src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs
Outdated
Show resolved
Hide resolved
|
||
// gRPC service used by Durable Task Framework (DTFx) backend implementations. | ||
// The RPCs in this service are used by DTFx backends to manage orchestration state. | ||
service BackendService { |
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'm hesitant to add a new proto just for this feature. Is it possible that the middleware for this should exist elsewhere?
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 share this concern, but I don't have good understanding of what alternatives are available. Let's list our options. To start:
- Can we add support for this feature to the exising Grpc project?
- Where can we move this middleware (so that it wouldn't require this gRPC update, if this is what Hal means)?
I'm not strictly against doing it the way it's currently implemented, but let's understand and compare our options.
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.
- Can we add support for this feature to the exising Grpc project?
Unfortunately not, dotnet cant generate same proto compiled types under multiple namespaces in same project assembly. we need two projects to generate proto types under two namespaces respectively. - Where can we move this middleware (so that it wouldn't require this gRPC update, if this is what Hal means)?
We can move the azuremanaged interceptor to azuremanaged sdk in DTMB repo . It would create coupling not make sense much to put the blob which is opt in extension logic like intereceptor into existing azuremanaged packages directly. if we move the middleware to there, we would still need a new extension package.
the other alternative is creating multiple packages:
Microsoft.DurableTask.Extensions.AzureBlobPayloads.Abstractions (common large payload apis including payloadstore, baseinterceptor,etc shared by other packages)
Microsoft.DurableTask.Extensions.AzureBlobPayloads.Sidecar (sidecar interceptorfor portable dotnet sdk connecting to sidecar grpc )
Microsoft.DurableTask.Extensions.AzureBlobPayloads.AzureManaged (azuremanaged intereceptor for df cases)
Hi
good point, i updated to shared option and store |
src/Extensions/AzureBlobPayloads/Examples/SharedPayloadStoreExample.cs
Outdated
Show resolved
Hide resolved
src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs
Outdated
Show resolved
Hide resolved
/// gRPC interceptor that externalizes large payloads to an <see cref="IPayloadStore"/> on requests | ||
/// and resolves known payload tokens on responses for Azure Managed Backend. | ||
/// </summary> | ||
public sealed class AzureBlobPayloadsManagedBackendInterceptor(IPayloadStore payloadStore, LargePayloadStorageOptions options) |
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.
Quite a lot of code duplication between this and AzureBlobPayloadsSideCarInterceptor
- wondering if there is a way to extract the common code to a shared component.
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.
will address once we confirm on the package approach
This pull request introduces support for externalizing large payloads to Azure Blob Storage in Durable Task applications. It adds a new extension package, configuration options, and a sample console app demonstrating the feature. The main changes include new abstractions for payload storage, Azure Blob Storage integration, and updates to the solution and dependencies.
Externalized Payloads Support:
IPayloadStore
interface inMicrosoft.DurableTask.Converters
to abstract storing and retrieving large payloads out-of-band.LargePayloadStorageOptions
for configuring payload externalization, including threshold, connection string, container name, and compression.AzureBlobPayloads
extension package (AzureBlobPayloads.csproj
) to provide Azure Blob Storage-based payload storage, with dependency injection support and gRPC integration. [1] [2]Azure.Storage.Blobs
,System.ComponentModel.Annotations
). [1] [2] [3] [4] [5]Sample Application and Documentation:
LargePayloadConsoleApp
sample, including project files, code demonstrating orchestration and entity scenarios with large payloads, and a README for setup and usage. [1] [2] [3] [4]