Get instance id for desired control-queue(s) #1069

pasaini-microsoft · 2024-04-19T20:35:38Z

Motivation

#1079

Issue: No way of targeting an orchestrator instance to a desired control-queue.

We have been facing issues where DTF orchestration used to get stuck at random. Given that customer load is not very regular in our service, it was challenging to understand upfront if the orchestration would be processed or will be stuck.
More often customers used to reach out with incidents complaining their request not completing for long time.
This is where we needed orchestration instances to observe health of each queue by targeting one instance for desired control-queue.

Motivation:

motivation was to reduce the TTD for finding if orchestration can be stuck/waiting-forever in a control-queue irrespective of the cause.

Issue: No way to load lightly loaded control-queues.

We have face a few situations where some of control-queues are overwhelmed with orchestration instances while the others are happily processing almost nothing.

Motivation:

motivation was to target new instances of orchestration instances to set control-queue which are not heavily loaded

Proposal

API to generate instance id for a set of control-queues.

This API receives set of control-queues and prefix for instance id.
Implementation detail is: Allow special way of creating instance id with a suffix unsigned integer after delimiter '!' and explicitly use that value to route to control-queue (say suffixNumber % partitionCount). If this pattern is not used, it would goes back to default (current) which is hash(instance-id)%partition-count.

davidmrdavid

Some thoughts

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs

davidmrdavid · 2024-07-08T18:42:17Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

+                var partitionId = index % totalPartitions;
+                return (uint)partitionId;


So this allows for the following scenario:
Assume we have 4 partitions,
And user creates instanceID "abc!7", they will still land on some queue, but it won't be the 7th queue, because that doesn't exist.

From first principles, I would think we'd want to error out in this case. But it seems this behavior is consistent with Netherite. I would prefer to throw in this case, but curious to know what others think.

@pasaini-microsoft - now that I think about it, it may be a good idea to keep the behavior you implemented. If in the future, we make the partitionCount something that users can change 'on the fly' (not possible today, but I'm working to make this happen), then this behavior would be resilient to changes in the number of partitions.

Let's keep this behavior for now but let's also try to emit a warning for when the total number of partitions is less than the customer's specified target number. That will help notify the customer that something possibly unintuitive is taking place. Thanks

I am actually in favor of not generating a warning. The reason is that the warning would actually fire almost constantly in all the applications where I have used this.

The expected use is that applications want to distribute things over the queues but dont actually know or care how many queues there are (e.g. like partition keys in Azure Storage).

I agree with @sebastianburckhardt - I feel this will be more noisy than helpful. I would only consider it if this leads to undefined behavior. But it isn't, it is by design hence the % operator. We need to make sure this behavior is well documented.

Ok, let's ignore the warning, I'm convinced. Agreed on the need to document it. We can do that documentation in an azure-docs PR.

…ettings.cs Co-authored-by: David Justo <[email protected]>

davidmrdavid · 2024-07-10T20:27:33Z

Test/DurableTask.AzureStorage.Tests/TestPartitionIndex.cs

+            controlQueueNumberToNameMap = new Dictionary<string, int>();
+
+            for (int i = 0; i < partitionCount; i++)
+            {
+                var controlQueueName = AzureStorageOrchestrationService.GetControlQueueName(settings.TaskHubName, i);
+                controlQueueNumberToNameMap[controlQueueName] = i;
+            }
+        }


are we still using this in the new tests? No, right?

davidmrdavid

Have we tested this for external events as well?

…https://github.com/pasaini-microsoft/durabletask into users/pasaini/main/InstanceIdForSelectedControlQueue

davidmrdavid · 2024-08-20T16:57:11Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs

+        /// <summary>
+        /// Whether to allow instanceIDs to use special syntax to land on a specific partition.
+        /// If enabled, when an instanceID ends with suffix '!nnn', where 'nnn' is an unsigned number, the instance will land on the partition/queue for to that number.
+        /// </summary>
+        public bool EnableExplicitPartitionPlacement { get; set; } = false;


Something to consider - it is not safe to change this from false to true (or vice-versa) while an orchestrator with the special syntax is in-flight. If we do that, any pre-existing messages for that orchestrator may now be considered to be "in the wrong queue".

Let's call this out in the intellisense

davidmrdavid · 2024-08-20T17:04:06Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

+
+            int placementSeparatorPosition = instanceId.LastIndexOf('!');
+
+            // if the instance id ends with !nnn, where nnn is an unsigned number, it indicates explicit partition placement


can we add a test that documents the behavior if the customer uses an instanceID with multiple ! in there? Say instanceID "A!1!B!3` should probably map to partition "3", right?

Let's also add a test that checks that instanceID myinstanceID!NotANumber does not trigger any errors / that it correctly ignores the explicit placement logic.

pasaini-microsoft added 3 commits April 19, 2024 12:42

Get instance id for desired control-queue(s)

409edba

Bringing randomization to instance ids.

2280605

Updating logic for consistenc with netherite.

e98bc4d

davidmrdavid reviewed Jul 8, 2024

View reviewed changes

pasaini-microsoft and others added 2 commits July 9, 2024 21:21

Updating tests

e9ee9c0

Update src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceS…

12719fa

…ettings.cs Co-authored-by: David Justo <[email protected]>

davidmrdavid reviewed Jul 10, 2024

View reviewed changes

pasaini-microsoft added 2 commits July 29, 2024 06:41

Addressing PR comments.

a07015d

Merge branch 'users/pasaini/main/InstanceIdForSelectedControlQueue' of …

f39724d

…https://github.com/pasaini-microsoft/durabletask into users/pasaini/main/InstanceIdForSelectedControlQueue

davidmrdavid reviewed Aug 20, 2024

View reviewed changes

Addressed comments.

cb9adbc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get instance id for desired control-queue(s) #1069

Get instance id for desired control-queue(s) #1069

pasaini-microsoft commented Apr 19, 2024 •

edited

Loading

davidmrdavid left a comment

davidmrdavid Jul 8, 2024

davidmrdavid Jul 8, 2024

sebastianburckhardt Jul 10, 2024

jviau Jul 10, 2024 •

edited

Loading

davidmrdavid Aug 20, 2024 •

edited

Loading

davidmrdavid Jul 10, 2024

davidmrdavid left a comment

davidmrdavid Aug 20, 2024

davidmrdavid Aug 20, 2024

davidmrdavid Aug 20, 2024

		var partitionId = index % totalPartitions;
		return (uint)partitionId;


		int placementSeparatorPosition = instanceId.LastIndexOf('!');

		// if the instance id ends with !nnn, where nnn is an unsigned number, it indicates explicit partition placement

Get instance id for desired control-queue(s) #1069

Are you sure you want to change the base?

Get instance id for desired control-queue(s) #1069

Conversation

pasaini-microsoft commented Apr 19, 2024 • edited Loading

Motivation

Issue: No way of targeting an orchestrator instance to a desired control-queue.

Motivation:

Issue: No way to load lightly loaded control-queues.

Motivation:

Proposal

API to generate instance id for a set of control-queues.

davidmrdavid left a comment

Choose a reason for hiding this comment

davidmrdavid Jul 8, 2024

Choose a reason for hiding this comment

davidmrdavid Jul 8, 2024

Choose a reason for hiding this comment

sebastianburckhardt Jul 10, 2024

Choose a reason for hiding this comment

jviau Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

davidmrdavid Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

davidmrdavid Jul 10, 2024

Choose a reason for hiding this comment

davidmrdavid left a comment

Choose a reason for hiding this comment

davidmrdavid Aug 20, 2024

Choose a reason for hiding this comment

davidmrdavid Aug 20, 2024

Choose a reason for hiding this comment

davidmrdavid Aug 20, 2024

Choose a reason for hiding this comment

pasaini-microsoft commented Apr 19, 2024 •

edited

Loading

jviau Jul 10, 2024 •

edited

Loading

davidmrdavid Aug 20, 2024 •

edited

Loading