Break down the orchestrator via strategies or partial class

[vany0114]

There might be some orchestrations kinda complex in terms of the workflow, interactions with external services, activities, etc, so in order to avoid having all that logic in the orchestrator trigger implementation and make it readable (avoid Spaghetti code), clear, and maintainable is it a good idea to break it down in several pieces using let's say factory/strategies or even implement it as a partial class? of course, as the documentation states, all the async calls would be the ones provided by the IDurableOrchestrationContext object, for example:

namespace Something
{
    public class Order
    {
        private readonly ILogger _logger;
        private readonly HandleOrderStrategyFactory _handleOrderStrategyFactory;
        private readonly HandleUserStrategyFactory _handleUserStrategyFactory;

        public Order(ILogger<Order> logger, HandleOrderStrategyFactory handleOrderStrategyFactory, HandleUserStrategyFactory handleUserStrategyFactory)
        {
            _logger = logger;
            _handleOrderStrategyFactory = handleOrderStrategyFactory;
            _handleUserStrategyFactory = handleUserStrategyFactory;
        }

        [FunctionName(Constants.ORDER_ORCHESTRATOR)]
        public async Task StartOrderOrchestrator([OrchestrationTrigger] IDurableOrchestrationContext context)
        {
            var @event = context.GetInput<SomeInput>();

            try
            {
                // Is it safe to call these async methods here w/o wrapping them into an Activity Function since internally it does so and they are deterministic?
                // plus the async operations they perform are in the IDurableOrchestrationContext which is safe.
                var handleUserStrategy = await _handleUserStrategyFactory.Get(@event, context);
                var userId = await handleUserStrategy.Handle(new HandleUserInput
                {
                    DurableContext = context,
                    ProviderOrderContext = @event
                });

                IHandleOrderStrategy handleOrderStrategy;
                var (orderEntityId, entityX, entityY) = BuildEntityIds(@event); // this method just returns a tuple of EntityId where ids are deterministic

                using (await context.LockAsync(orderEntityId, entityX, entityY))
                {
                    await InitializeEntities(context, @event, orderEntityId, entityX, entityY); // this method call the entities using the CallEntityAsync method

                    handleOrderStrategy = await _handleOrderStrategyFactory.Get(@event, context);
                    await handleOrderStrategy.Handle(new HandleOrderInput
                    {
                        DurableContext = context,
                        OrderEntityId = orderEntityId,
                        EntityX = entityX,
                        EntityY = entityY,
                        UserId = userId
                    });
                }
            }
            catch (Exception ex)
            {
                context.CreateReplaySafeLogger(_logger).LogError(ex, "Unhandled error while executing {orchestrator} from order {orderId}", Constants.ORDER_ORCHESTRATOR, orderId);
            }
        }
    }
}

Factory example:

namespace Something
{
    public class HandleUserStrategyFactory
    {
        protected string _userId;
        protected readonly ILogger<HandleUserStrategyFactory> _logger;
        protected readonly IEventBus _eventBusClient;
        protected readonly IApiGateway _apiGatewayClient;

        public HandleUserStrategyFactory(ILogger<HandleUserStrategyFactory>, IEventBus eventBusClient, IApiGateway apiGatewayClient)
        {
            _logger = logger;
            _eventBusClient = eventBusClient;
            _apiGatewayClient = apiGatewayClient;
        }

        [Deterministic]
        public async Task<IHandleUserStrategy> Get(SomeInput providerOrderContext, IDurableOrchestrationContext durableContext)
        {
            if (x || y)
                return new NoOpUserStrategy(_logger, _eventBusClient, _apiGatewayClient);

            var retryOptions = new RetryOptions(TimeSpan.FromSeconds(1), 3);
            var user = await durableContext.CallActivityWithRetryAsync<UserDto>("GetUser", retryOptions, providerOrderContext);

            _userId = user?.Id;
            if (string.IsNullOrEmpty(_userId))
                return new CreateNewUserStrategy(_logger, _eventBusClient, _apiGatewayClient);
            else if (string.IsNullOrEmpty(_userId) == false)
                return new UpdateExistingUserStrategy(_logger, _eventBusClient, _apiGatewayClient, _userId);

            throw new InvalidOperationException($"Not able to determine the IHandleUserStrategy, {providerOrderContext.SourceIdentities.Order}");
        }

        [FunctionName("GetUser")]
        public async Task<UserDto> GetUserMapping([ActivityTrigger] SomeInput input)
        {
            return await _apiGatewayClient.GetUserIdBySourceIdentity(input);
        }
    }
}

Strategy example:

namespace Something
{
    /// <summary>
    ///  This strategy creates new users.
    /// </summary>
    public class CreateNewUserStrategy : HandleUserStrategyFactory, IHandleUserStrategy
    {
        public CreateNewUserStrategy(ILogger<HandleUserStrategyFactory> logger, IEventBus eventBusClient, IApiGateway apiGatewayClient)
            : base(logger, settings, eventBusClient, apiGatewayClient)
        {
        }

        [Deterministic]
        public async Task<string> Handle(HandleUserInput input)
        {
            var userId = string.Empty;
            var durableContext = input.DurableContext;
            var retryOptions = new RetryOptions(TimeSpan.FromSeconds(1), 3);

            await durableContext.CallActivityWithRetryAsync("CreateNewUser", retryOptions, input);

            using var cts = new CancellationTokenSource();
            var timeout = durableContext.CurrentUtcDateTime.AddSeconds(_settings.Value.SAGA_USER_TIMEOUT_IN_SECONDS);
            var timeoutTask = durableContext.CreateTimer(timeout, cts.Token);
            var userCreatedTask = durableContext.WaitForExternalEvent<UserCreatedIntegrationEvent>(Constants.USER_CREATED_EXTERNAL_EVENT);
            var invalidPhoneTask = durableContext.WaitForExternalEvent<InvalidUserPhoneNumberIntegrationEvent>(Constants.USER_INVALID_PHONE_NUMBER_EXTERNAL_EVENT);

            var winner = await Task.WhenAny(userCreatedTask, invalidPhoneTask, timeoutTask);
            if (winner == userCreatedTask)
            {
                cts.Cancel();
                userId = userCreatedTask.Result.UserId;
                durableContext.CreateReplaySafeLogger(_logger).LogInformation($"User created, User id: {userId}, Order id: {input.OrderId}, Phone number: {newUserIntegrationEvent.Normalization.PhoneNumber}.");
            }
            else if (winner == invalidPhoneTask)
            {
                cts.Cancel();
                durableContext.CreateReplaySafeLogger(_logger).LogError($"User wasn't created, Order id: {input.OrderId}, Phone number: {newUserIntegrationEvent.Normalization.PhoneNumber}, Reason: {invalidPhoneTask.Result.ErrorMessage}. Order will be created w/o user.");
            }
            else
            {
                durableContext.CreateReplaySafeLogger(_logger).LogError($"Timeout trying to create the user, Order id: {input.OrderId}, Phone number: {newUserIntegrationEvent.Normalization.PhoneNumber}. Order will be created w/o user.");
            }

            return userId;
        }

        [FunctionName("CreateNewUser")]
        public async Task CreateNewUser([ActivityTrigger] NewProviderUserIntegrationEvent integrationEvent)
        {
            _logger.LogInformation($"Creating user for Order: {integrationEvent.OrderId}, Provider: {integrationEvent.Metadata.Provider}, Phone number: {integrationEvent.Normalization.PhoneNumber}");

            await _eventBusClient.Publish(
               integrationEvent,
               nameof(NewProviderUserIntegrationEvent),
               Constants.EVG_EVENT_SUBJECT_INTEGRATION,
               integrationEvent.Metadata.Traceparent,
               _settings.Value.NEW_PROVIDER_USER_LOCAL_ENDPOINT
           );
        }
    }
}

As you can see the orchestrator trigger implementation is simple because all the logic is broken down into strategies, you can imagine that having all the strategies code in the orchestrator trigger implementation would be quite convoluted. Do you guys see any risk/consideration with this approach in terms of misbehaving at runtime? I'm asking because while this works like a charm locally we've had issues with the production environment mostly with instances getting stuck, particularly when I switched to the new partition manager.

Another way to implement it would be via partial classes rather than breaking it down into strategies.

cc: @cgillum @sebastianburckhardt

[vany0114]

@cgillum @sebastianburckhardt bumping this as a friendly reminder, I'd like to get your thoughts on this.

[nytian]

Hi, I am sorry for the trouble. Can you provide me the orchestratior instanceId that had this issue, with region, and app name? I can query it to see what happened. Thanks!

[vany0114]

Unfortunately, I had to reset everything (by removing the func app' storage account, the durable functions' storage account, and the func app itself and then re-provision everything again) so I don't have any of those logs. I'm pretty sure it will happen again soon...in the meantime do you guys see any issue, concern, or consideration with the illustrated approach?

[nytian]

Ok. The approach looks good to me. We can wait now until the error is reproduced, and I will query it to check the issue.

@jviau Jacob, I wonder can you help take a look at the customer's code as you have more in-depth knowledge of .Net in case I missed anything? Thank you!

[vany0114]

@jviau Jacob, I wonder can you help take a look at the customer's code as you have more in-depth knowledge of .Net in case I missed anything? Thank you!

That'd be great!

[jviau]

I don't see anything that stands out with the code. Given the later comment from @vany0114, the issue appears to be with entity locks anyways - which I am unfamiliar with.

Not sure if you considered this, but you can also leverage sub-orchestrations for encapsulation. For example, you could have a sub-orchestration GetOrCreateUser, which does the strategy work of calling an activity to see if a user exists (returning if it does) and creating then returning a user if it does not.

Sub orchestrations also have the benefit of breaking your orchestration history down into trees, which makes replays ever so slightly more efficient as now we only need to replay a single level of the tree.

[vany0114]

Ok it's happening again, here is some information that will let you guys look into it:

• {"checkNumber":"30001","storeId":"644d72b6-2ecc-4db3-b732-c3a24579fa35","id":"00300001-06bd-0000-1708-070000000001"} instance was stuck 18 minutes and it seems it was acquiring the lock, these are the entities locked when orchestration starts:

  • @ticketnumberkey@644d72b6-2ecc-4db3-b732-c3a24579fa35_2023-08-07_30001
  • @originorderkey@644d72b6-2ecc-4db3-b732-c3a24579fa35_2023-08-07_30001
  • @scheduledkey@644d72b6-2ecc-4db3-b732-c3a24579fa35_2023-08-07_30001
  • @order@{"checkNumber":"30001","storeId":"644d72b6-2ecc-4db3-b732-c3a24579fa35","id":"00300001-06bd-0000-1708-070000000001"}
    

• {"guid":"292463b3-c900-49ae-8686-ed842b45ead3","storeGuid":"5df75b4b-d42b-4ec7-b6fd-63815b332beb"} same issue as above, the instance was stuck 20 minutes while acquiring the lock.

  • @ticketnumberkey@82534e00-a73d-4eef-9912-d13cc4e94d78_2023-08-07_1
  • @originorderkey@82534e00-a73d-4eef-9912-d13cc4e94d78_2023-08-07_1
  • @scheduledkey@82534e00-a73d-4eef-9912-d13cc4e94d78_2023-08-07_1
  • @order@{"guid":"292463b3-c900-49ae-8686-ed842b45ead3","storeGuid":"5df75b4b-d42b-4ec7-b6fd-63815b332beb"}
    

• {"storeId":"615c29df-5e7c-458c-9486-ef9480d2bf3b","courseUid":"c77bb860-04f7-46d3-86d9-d8c0e65fe856","checkNumber":"90007"} same, the instance was stuck 21 minutes:

  • @ticketnumberkey@615c29df-5e7c-458c-9486-ef9480d2bf3b_2023-08-07_90007-1
  • @originorderkey@615c29df-5e7c-458c-9486-ef9480d2bf3b_2023-08-07_90007-1
  • @scheduledkey@615c29df-5e7c-458c-9486-ef9480d2bf3b_2023-08-07_90007-1
  • @order@{"storeId":"615c29df-5e7c-458c-9486-ef9480d2bf3b","courseUid":"c77bb860-04f7-46d3-86d9-d8c0e65fe856","checkNumber":"90007"}
    

There are more examples just in case you guys needed but I guess this is enough to illustrate what's happening, I'll open a bug as well since this is happening in our production environment, so we really appreciate your help asap on this.

Note: FWIW the latency gets worse as time goes by, I've seen hours of latency when it starts getting stuck.

Azure resources:

• Func app: orders-saga
• Stoarge account name: ordersagav2
• Location: West US

[davidmrdavid]

Thanks @vany0114. I'll follow up in the GitHub issue (#2534) so that this thread can be about your partial classes question, whereas the issue will be about investigating this latency/stuckness issue.