-
Notifications
You must be signed in to change notification settings - Fork 48
Support cancellation of web api and audit storage operations #4782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@danielmarbach @andreasohlund There are multiple commits, one for audit, one for error, and one for domain events. Issue it that seems like a very deep rabbit hole as many handlers invoke storage and all of these do not support cancellation. As cancellation itself isn't really a bug but an improvement. As audit is the most affected I think it makes sense to support cancellation there and for the remaning parts to create cruft issues as it is a lot of effort to add cancellation to all those operations. What is your opinion? |
FYI During the modernization efforts we deliberately had to stop at some point. It was just too much effort and also not in all cases glaringly obvious what the effects of cancellation might have on the stability of the code paths calling it so we did cut off somewhere. |
b779f00 to
b8d5744
Compare
4fb3818 to
825bc8f
Compare
…Source` with `RavenAuditIngestionUnitOfWork`
…but pretty much all storage API's aren't supporting cancellation....
# Conflicts: # src/ServiceControl.Audit/Auditing/AuditIngestion.cs # src/ServiceControl.Audit/Auditing/AuditIngestor.cs
…SagaSnapshot and RecordKnownEndpoint
…ady cancelled token. This should terminate ASAP BUT still cleanup all resources if properly implemented by transports/persisters.
08e1059 to
011ed4e
Compare
…he cancelled state
When ingestion hangs and the service looks unresponse a user might want to stop the instance. Especially for the batch ingestion is makes sense to support cancellation as this is a long running operations.
This is a port and continuation of my investigation PR: