Skip to content
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

Cancellable handler #549

Closed
wants to merge 7 commits into from
Closed

Cancellable handler #549

wants to merge 7 commits into from

Conversation

martincostello
Copy link
Member

I had an idea this morning based on the changes in #546, so I thought I'd see how far I could get with it and submit a PR for discussion.

In the event that the issue that lead to #546 occurring and tasks to process messages back up, it's possible that a task to process a message may be trying to process a message that was received so long ago that the message visibility timeout has expired and it is now a potential duplicate waiting to be processed.

If the handler had a CancellationToken that cancels after the approximate message visibility timeout expires, JustSaying could enable two behaviours:

  1. It could self-cancel trying to do anything with the message before getting to the handler and abandoning it (potentially just extending the visibility instead, but I've not done that here).
  2. The handler, when called, could observe the cancellation token itself and make a decision about what to do with the message. It could return false to send it back to the queue (where it could have since been handled elsewhere anyway), or it could return true on the basis, if this has expired I'll just ignore it and pretend I've dealt with it.

This is implemented here by adding the ICancellableHandlerAsync<T> interface that derives from IHandlerAsync<T>, and then plumbing it in appropriately so it's used if implemented by the handler for a particular message and extending things like the exactly-once handler and stopwatch wrappers.

There's also some random bits of refactoring in here that happened as I was going along, I'll pull those out into a separate PR shortly.

Improve the behaviour of the Throttled implementation to use the async methods on the SemaphoreSlim class rather than wrap wait handles in tasks and manually queue to the thread pool. Also makes changes to consider the fact that code using the processing strategy can race and think there are available workers but then there not be any available when it attempts to process the message.
Remove leftover code from previous refactors.
Simplify the message loop and remove the concept of AvailableWorkers from IMessageProcessingStrategy.
Refactor the message loop to make it easier to follow.
Remove AvailableWorkers property.
Rename MaxWorkers to MaxConcurrency.
Add ThrottledOptions class.
Support serial processing as well as fire-and-forget using the thread pool.
Try and fix integration tests failing if tasks transition so they're already completed.
Update various NuGet pacakges to their latest versions.
Add a new concept of a handler that can be cancelled if the message visibility timeout has expired.
@stale
Copy link

stale bot commented Aug 25, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@martincostello
Copy link
Member Author

Replaced by #571 as this PR appears to be broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant