Allow handler execution time to exceed the default transaction timeout #661
Conversation
danielmarbach
requested review from
abparticular,
lailabougria and
ramonsmits
|
Wouldn't it be possible to manage the TX in |
danielmarbach
force-pushed
the
lazy-transaction
branch
from
642c793
to
3244abb
Compare
danielmarbach
force-pushed
the
lazy-transaction
branch
from
3244abb
to
449cd94
Compare
|
@ramonsmits Dispatch will be called potentially multiple times. For example, when you have immediate dispatches that will call the dispatcher directly while you still might be collection other non-immediate dispatches. |
Also has less closure allocations and doesn't lock
|
I thought I found an elegant approach until I realized the dispatcher still needs to get out the ServiceBusClient, the partition key and the committable transaction. Otherwise, it would break azure functions |
|
Looking at all the test cases and the conditional complexity within the AzureServiceBusTransaction class I'm coming to the conclusion it is better to make that concept public. Functions already references the transport of a given version and they kind of depend on each other. That would be the cleaner approach. |
danielmarbach
changed the title
|
@SeanFeldman in case you are curious, I would love to get your input on this one. Of course, I don't want to overload you, so feel free to ignore in case you don't have time. |
Simplify conditional logic Co-authored-by: Andreas Bednarz <110360248+abparticular@users.noreply.github.com>
danielmarbach
force-pushed
the
lazy-transaction
branch
from
bfa9ecc
to
70f104d
Compare
|
Set it to auto-merge since I think I have adressed the comments |
…ction timeout (#661) * Open the committable transaction as late as possible * Custom ReceiveTransaction for better encapsulation Also has less closure allocations and doesn't lock * Change approach slightly including some value caching * TODO for the implementation * Add tests for all the complex cases * Simplify approach by making the transaction public * Adjust transaction handling to expose the base class Simplify conditional logic Co-authored-by: Andreas Bednarz <110360248+abparticular@users.noreply.github.com> * Adjust AzureServiceBusTransportTransaction documentation Co-authored-by: Andreas Bednarz <110360248+abparticular@users.noreply.github.com>
danielmarbach
changed the title
This PR introduces an AzureServiceBusTransportTransaction class that encapsulates the management of the client, partition key and the committable transaction. This allows us to defer the creation of the committable transaction object to the point when we really require it (when dispatching non-isolated dispatches). Due to the lazy creation, the transaction timeout only applies to the point in time when we start dispatching messages and not to the time when we start handling the message. This means the pipeline execution time no longer needs to complete within the transaction timeout and can take as long as Azure Service Bus allows, keeping the message under lock.
During the development of this change I have evaluated several options that would not require Azure Functions integration to be changed but ultimately decided the complexity of the inner workings are not worth it especially considering the fact that Azure Function integration for Azure Service Bus already has a dependency on the transport. On top of that having the transaction public and self-registering in the transport transaction makes the integration point slightly more explicit than just relying on three values being explicitly put into the transport transaction (and one using a magic key).