Azure / azure-functions-durable-extension

Durable Task Framework extension for Azure Functions
MIT License
717 stars 271 forks source link

Breaking behavior change in DurableClient resolution between v2.5.1 to v2.7.1 #2336

Open cgillum opened 1 year ago

cgillum commented 1 year ago

Discussed in https://github.com/Azure/azure-functions-durable-extension/discussions/2335

Originally posted by **siemenstan** December 11, 2022 I try to upgrade Durable Function library from v2.5.1 to v2.7.1 but my app doesn't work the same like before after the library upgrade. May I know if there any behavior change or do I need to update my configuration? Below is a simple diagram that illustrate my app running in AKS, there is an API AKS node which handle and delegate the requests to 3 Execution AKS node, through 3 azure storage accounts, each execution node deal with its respective storage account. The storage account will have the same suffix name, eg: AzureWebStorage**02b** will point to StorageAccountName**02b** ![image](https://user-images.githubusercontent.com/98590735/206953802-f5957a67-223a-4e94-80b0-3867c42c4b0d.png) The API node will call the IDurableClientFactory.CreateClient(...) method to get the durable client, and then call the IDurableClient.StartNewAsync(...) to start the execution orchestration function, below is the code snippet ``` IDurableClient orchestrationClient = this.durableClientFactory.CreateClient(new DurableClientOptions { ConnectionName = connectionName, TaskHub = this.configurationProvider.TaskHubName, }); orchestrationClient.StartNewAsync(SharedFunctionConstants.ExecuteActionOrchestratorName, instanceId, orchestrationInput); ``` The durable client options connection name will be one of the 3 values: AzureWebStorage02 / AzureWebStorage02b / AzureWebStorage02c, set in AKS pod environment variable, that contains azure storage account storage connection string. All this while with Durable Function v2.5.1, this work fine, eg: with ConnectionName of AzureWebStorage02b, the request will go to StorageAccount02b, and Execution-2 node will pick up and execute the job. This also works fine with Durable Function v2.6.1, but not with Durable v2.7.1 or later (didn't test v2.7.0 due to .net6 dependancy issue). All the requests only write to the StorageAccount02 (which is pointed by variable 'AzureWebStorage02' or 'AzureWebStorage') Is there any behavior changes or configuration changes required which doesn't mention in release note? (Here is the simplified API AKS node environment print: [API env.txt](https://github.com/Azure/azure-functions-durable-extension/files/10204454/API.env.txt)) Below is the instance with Durable Function v2.8.1 on 10-Dec-2022 03:20AM UTC. The request suppose to write to **AzureWebJobStorage02b** (StorageAccount02b, and execute by Execution-2 pod) but it actually write to **StorageAccount02** (and execute by Execution-1) Orchestration InstanceId: ce57ffef-6826-498b-8cf1-dca80aab1e28 And, below is the instance with Durable Function v2.5.1 on 12-12-Dec 03:02AM UTC. The request write to AzureWebJobStorage02b (StorageAccount02b, and execute by Execution-2 pod) OrchestrationInstanceId d947f0e4-7475-45f2-b8b0-e361823fce0e Your help is very much appreciated.
bachuv commented 1 year ago

Updating here that there was a breaking change in v2.7.0 where connection string information was moved into StorageAccountDetails. The fix for this issue is to also check StorageAccountDetails for the connection string information in GetAzureStorageStorageProvider(). I will open a PR for this shortly.

siemenstan commented 8 months ago

Hi @bachuv @cgillum , We recently upgraded to version v2.13.0 and notice this bug still exists. May I know what is the plan for this bug?

HanChong77 commented 8 months ago

We ran into a similar issue as well. Hope this bug can be addressed soon, so that we can upgrade to the latest version.

bachuv commented 7 months ago

Just wanted to share an update that I'm investigating this issue and will probably need to take a different approach than the one described in a previous comment. I'll share an update next week.

siemenstan commented 6 months ago

Good day @bachuv , may I know if you have any updates or new approach that you could share with us? Would like to know if there is any code changes required on our side with your new approach. Hope to hear from you soon, thank you!

bachuv commented 6 months ago

As an update, we are prioritizing this fix to go in the next Durable Functions release. We don't have an ETA for that yet, but I'll share an update here when that's decided.

marvin-tan commented 5 months ago

hi @bachuv can I please check with you if there is any update on when this fix will be released? thanks!

hctan commented 1 month ago

hi @bachuv , @cgillum , I've made a change (mentioned earlier in the thread) & tested the fix. This is the PR , can you help to review it?

After the change in version 2.7.1, the StorageConnectionString is no longer being assigned any value, and the setting is only configured in GetAzureStorageOrchestrationServiceSettings() function, so I think should be safe to just check for StorageAccountDetails -> ConnectionString