From be33048169444630c1d80377ad03a14f58fbbc89 Mon Sep 17 00:00:00 2001
From: Sebastian Burckhardt <sburckha@microsoft.com>
Date: Thu, 29 Oct 2020 16:13:54 -0700
Subject: [PATCH] address PR feedback.

---
 .../ContextImplementations/DurableClient.cs    | 11 ++++++++---
 .../HttpApiHandler.cs                          | 18 ++++++++++++++----
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs
index d4d6d1140..c8f5a8b57 100644
--- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs
+++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs
@@ -71,6 +71,7 @@ internal class DurableClient : IDurableClient,
 
         string IDurableEntityClient.TaskHubName => this.TaskHubName;
 
+        /// <inheritdoc/>
         public override string ToString()
         {
             return $"DurableClient[backend={this.config.GetBackendInfo()}]";
@@ -534,10 +535,14 @@ async Task<CleanEntityStorageResult> IDurableEntityClient.CleanEntityStorageAsyn
                          tasks.Add(CheckForOrphanedLockAndFixIt(state, status.LockedBy));
                     }
 
-                    if (removeEmptyEntities && !status.EntityExists && status.LockedBy == null && status.QueueSize == 0
-                        && now - state.LastUpdatedTime > this.config.MessageReorderWindow)
+                    if (removeEmptyEntities)
                     {
-                        tasks.Add(DeleteIdleOrchestrationEntity(state));
+                        bool isEmptyEntity = !status.EntityExists && status.LockedBy == null && status.QueueSize == 0;
+                        bool safeToRemoveWithoutBreakingMessageSorterLogic = now - state.LastUpdatedTime > this.config.MessageReorderWindow;
+                        if (isEmptyEntity && safeToRemoveWithoutBreakingMessageSorterLogic)
+                        {
+                            tasks.Add(DeleteIdleOrchestrationEntity(state));
+                        }
                     }
                 }
 
diff --git a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs
index fca091437..4cf7cac5d 100644
--- a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs
+++ b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs
@@ -167,12 +167,16 @@ private static TemplateMatcher GetInstanceRaiseEventRoute()
 
             IDurableOrchestrationClient client = this.GetClient(request);
             Stopwatch stopwatch = Stopwatch.StartNew();
+
+            // This retry loop completes either when the
+            // orchestration has completed, or when the timeout is reached.
             while (true)
             {
                 DurableOrchestrationStatus status = null;
 
                 if (client is DurableClient durableClient && durableClient.DurabilityProvider.SupportsPollFreeWait)
                 {
+                    // For durability providers that support efficient (poll-free) waiting, we take advantage of that API
                     try
                     {
                         var state = await durableClient.DurabilityProvider.WaitForOrchestrationAsync(instanceId, null, timeout, CancellationToken.None);
@@ -180,10 +184,14 @@ private static TemplateMatcher GetInstanceRaiseEventRoute()
                     }
                     catch (TimeoutException)
                     {
+                        // The orchestration did not complete.
+                        // Depending on the implementation of the backend, we may get here before the full timeout has elapsed,
+                        // so we recheck how much time has elapsed below, and retry if there is time left.
                     }
                 }
                 else
                 {
+                    // For durability providers that do not support efficient (poll-free) waiting, we do explicit retries.
                     status = await client.GetStatusAsync(instanceId);
                 }
 
@@ -717,13 +725,15 @@ private static bool TryGetIntQueryParameterValue(NameValueCollection queryString
                 TimeSpan? timeout = GetTimeSpan(request, "timeout");
                 TimeSpan? pollingInterval = GetTimeSpan(request, "pollingInterval");
 
-                if (timeout.HasValue && pollingInterval.HasValue)
+                // for durability providers that support poll-free waiting, we override the specified polling interval
+                if (client is DurableClient durableClient && durableClient.DurabilityProvider.SupportsPollFreeWait)
                 {
-                    return await client.WaitForCompletionOrCreateCheckStatusResponseAsync(request, id, timeout.Value, pollingInterval.Value);
+                    pollingInterval = timeout;
                 }
-                else if (timeout.HasValue && client is DurableClient durableClient && durableClient.DurabilityProvider.SupportsPollFreeWait)
+
+                if (timeout.HasValue && pollingInterval.HasValue)
                 {
-                    return await client.WaitForCompletionOrCreateCheckStatusResponseAsync(request, id, timeout.Value, timeout.Value);
+                    return await client.WaitForCompletionOrCreateCheckStatusResponseAsync(request, id, timeout.Value, pollingInterval.Value);
                 }
                 else
                 {