Closed ErikMogensen closed 7 months ago
Looking good, @ErikMogensen. A few questions/suggestions.
@SeanFeldman, there were quite a few changes since your review. Do you want to take another look?
When I limited it to 40 parallel tasks it is quite fast and works 😄
I also added a new context menu item for selecting all rows
Selected a single, normal messages:
Selected multiple, normal messages:
Selected a single, dead-letter messages:
Selected multiple, deadletter messages:
Some thoughts about this PR in light of some benchmarking that was done.
It is worth mentioning that this is related to #360. We closed the issue as won't fix
because, with any significant number of scheduled messages, this approach (PeekLock and complete) might fail or take a long time. For example, running outside of an Azure data center and cancelling 500K messages of average size can take anywhere between 1 and 1.5 hours.
I also benchmarked an alternative approach, which is more intrusive, using auto-forwarding and a temporary queue. Assuming there's a source
queue with scheduled and active messages, auto-forwarding is set to a temporary queue temp
. Active messages are drained to the temporary temp
queue. At that point, the original source
queue is deleted and recreated (a short delay is required to avoid resource creation conflict error). And the last step is to forward backed-up messages from the temp
queue back to the newly re-created source
, deleting the temporary temp
queue. With a similar number of scheduled messages and 1000 active messages to emulate in-flight messages, it took under 30 seconds (manual verification).
While I still rather see ASB supporting purging (scenario when there are no inflight messages or those don't matter) and ability to peek messages by status, for large quantities of scheduled messages, the auto-forwarding option might be better suited. Food for thought
Fixed it to build and handle failed cancellations. Tested with 100.000 messages and it took a bit less than ten minutes to cancel all of them.
@SeanFeldman, interesting solution. What do you mean by:
this approach (PeekLock and delete)
I meant PeekLock and complete. The alternative is too destructive and would mess up anything that relies on the resource ID. Like monitoring and alerting. That’s why it really should be a service side capability (https://github.com/Azure/azure-service-bus/issues/698).
I meant PeekLock and complete.
Do you mean the solution I created and you just reviewed? It is using Peek to find out status and then CancelScheduledMessages on the scheduled messages.
Do you mean the solution I created and you just reviewed? It is using Peek to find out status and then CancelScheduledMessages on the scheduled messages.
Yes. There's no other option today. Except the one I mentioned but it's not "safe".
@SeanFeldman, do you approve of this PR now?
@SeanFeldman, do you approve of this PR now?
👍
Fixes #360
This PR adds the ability to cancel scheduled messages on queues. The Service bus API does not support browsing scheduled messages on subscriptions.
A new context menu item "Cancel Scheduled Message" has been added as shown below:
If multiple messages are selected it says "Cancel Scheduled Messages" instead:
If the Disable Accidental Deletion Prevention is disabled, a message box will ask for confirmation.
The rows in the grid will not update, nor will the message count for the queue in the tree view. That is something that may be added later.