Particular / NServiceBus.Transport.AzureServiceBus

Azure Service Bus transport
Other
22 stars 19 forks source link

Add LRU array and update exception handling for `receiveOnly` #1031

Closed TravisNickels closed 2 months ago

TravisNickels commented 3 months ago

This PR is a follow up related to:

✔ What has been done

💭 Assumptions

Applies only to receiveOnly

⏰ Time Complexity

The big-Θ is linear with respect to N and D/M

big-Θ (N * (ceil (D / M)))

N = number of receiver instances that could handle the message D = the time it takes for the handler to finish M = the max message lock duration and max auto lock renewal

Example:

N = 2 (number of instances) D = 33 seconds M = 5 seconds

ceil(33/5) = ceil(6.6) = 7 big-Θ (2 * 7) = big-Θ (14)

This indicates that in the worst-case scenario (big-Θ) across all the receiver instances, the handler could possibly execute 14 times before the message is finally removed from the queue.

In the best-case scenario (big-Ω) using the same example above, we could get lucky, and the same instance could handle the message, and the other instance never sees it. However, since the handler still takes longer than the message lock duration, the best-case would still only be big-Ω(7).

This means that it's still important to make sure that the handler execution time is less than or equal to the message lock duration and renewal to reduce big-Θ.

danielmarbach commented 3 months ago

Generally I'm a bit concerned about this change due to the following reasons

I have created a draft PR for discussion that shows some pseudocode and tests that show how things could be tackled. By moving the concrete scenarios closers to the complete and abandon it is possible to make targeted comments and log statements that make it clearer what's going on.

But even in that version, it becomes apparent that the current pump design might be suboptimal and might need to be revisited if we don't want to suffer through a series of bugs or behavior breaking changes by introducing this. I have a hunch it might be warranted to create some sort of receive strategy that is replaced depending on the transport transaction mode allowing to clearly separate the state tracking and error behavior for receive only vs sends with atomic receive

I think it is useful to closely investigate the current flow of complete, abandon and exception tracking and then align it closely on a case by case basis while using that knowledge to reconsider the logic flow of the pump

danielmarbach commented 2 months ago

Replaced by https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/pull/1034