Particular / NServiceBus.Transport.AzureServiceBus

Azure Service Bus transport
Other
22 stars 19 forks source link

expose AutoDeleteOnIdle setting #358

Open steve-haar opened 3 years ago

steve-haar commented 3 years ago

Is your feature request related to a problem? Please describe. When using azure cloud machines with AzureServiceBus new queues are created whenever a new cloud instance is brought online. When cloud instances are decommissioned the queues remain even though they will never be used again.

Describe the solution you'd like Expose the AutoDeleteOnIdle property in AzureServiceBusSettings so we can set this property on our queues and topics.

Describe alternatives you've considered The best alternative is to create a task/script that will detect and delete unused queues.

Additional context Here is a patch that I think would solve this.

0001-add-AutoDeleteOnIdle-setting.patch.txt

SeanFeldman commented 3 years ago

Hi @steve-haar,

If you want to suggest a change, opening a PR is a common way to suggest changes. But before we get into the changes discussion, could you please clarify a few things first?

When using azure cloud machines with AzureServiceBus new queues are created whenever a new cloud instance is brought online. When cloud instances are decommissioned the queues remain even though they will never be used again.

  1. Are you referring to Azure VMs, VMSS, or something else?
  2. When you provision an endpoint with "cloud machine", what drives the logical endpoint name? Is it deterministic or not?
  3. How a new cloud instance is brought online? Is that manual, automated, scripted?

Thank you.

steve-haar commented 3 years ago

Sure thing. Here is a PR: https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/pull/359.

  1. The situation arises for us because we are hosting NServiceBus endpoints in Azure app services and we are also marking the instances as uniquely addressable (to support callbacks). To make the endpoints uniquely addressable we use the azure environment variable WEBSITE_INSTANCE_ID as the discriminator.

  2. Because we use WEBSITE_INSTANCE_ID as the discriminator, the name is not deterministic.

  3. Azure app services will automatically create and destroy instances of the app service behind the scenes. New instances are created with a random name. Whenever an instance is created, it will create a new queue when it comes online. However, when an instance is destroyed, its queue will stick around forever.

SeanFeldman commented 3 years ago

Azure app services will automatically create and destroy instances of the app service behind the scenes. New instances are created with a random name. Whenever an instance is created, it will create a new queue when it comes online. However, when an instance is destroyed, its queue will stick around forever.

Could you please point to the documentation (if available) or describe how the tearing down of your App Service webapp is done? The reason I'm asking is that this is an infrastructure concern that could be potentially addressed w/o an update to the transport. Thank you.

steve-haar commented 3 years ago

We run our web api on an azure app service and often scale up or out during the course of the week. Whenever we scale up or sometimes just periodically, it will destroy and/or create new instances of the app service. This is all behind the scenes in azure and is not seen except that the WEBSITE_INSTANCE_ID changes so we know it's running on a new instance. Because we use callbacks we set the endpoint name discriminator to WEBSITE_INSTANCE_ID (equivalent of machine name in azure app services). At the end of the month we are left with hundreds of dead queues that reference old WEBSITE_INSTANCE_IDs that will never be used again.

I noticed that there is a AutoDeleteOnIdle setting in azure, but it looks like it can only be configured during queue creation. Our infrastructure team asked us if we could set that property on creation. When I went looking, I realized that the property is set (or rather not set), in the code for this package and it's not exposed.

I noticed that this property is exposed in the legacy WindowsAzureServiceBus (https://docs.particular.net/transports/azure-service-bus/legacy/configuration/full).

This could be solved by infrastructure, but it would involve a script that runs periodically to detect and remove unused/idle queues. However, detecting queue idleness is not trivial.

SeanFeldman commented 3 years ago

This could be solved by infrastructure, but it would involve a script that runs periodically to detect and remove unused/idle queues. However, detecting queue idleness is not trivial.

My intent was not to use scripting. Rather, I was thinking about when the webapp is about to stop (on the scale in) to have the code that is responsible for either delete the queue or mark is as to be deleted on idling. For example:

public class Startup
{
    public void Configure(IApplicationBuilder app, IApplicationLifetime applicationLifetime)
    {
        applicationLifetime.ApplicationStopping.Register(OnShutdown);
    }

    private void OnShutdown()
    {
         //this code is called when the application stops to delete the queue or mark is as to be deleted when idling
    }
}

For a IHostedService service, it would be in the StopAsync() method. Thoughts?

steve-haar commented 3 years ago

That's a good idea. I didn't think about deleting the queue on shutdown. This would probably in most cases. Some cases where this might not work would be if the app service had an ungraceful shutdown or if the asynchronous delete queue operation didn't complete in the time limit allocated for graceful shutdown.

This is an alternative, but I would prefer to set the native azure setting rather than adding shutdown code to make sure it worked properly all the time.

SeanFeldman commented 3 years ago

Some cases where this might not work would be if the app service had an ungraceful shutdown or if the asynchronous delete queue operation didn't complete in the time limit allocated for graceful shutdown.

To alter my own suggestion, you could change when the queue is modified and perform the necessary update upon the startup. You could even create the queue manually, just before the NServiceBus endpoint is started, and NServiceBus would keep it as-is to respect the pre-existing settings. That way, you're ensuring the queue is created the way you need it and that it will be removed when idling, no matter if the web app has shut down gracefully or not, always resulting in the queue deletion.

steve-haar commented 3 years ago

That's a good solution and could work for us. I would have to carefully look at NServiceBus source and understand all of the other settings when creating a queue. I would want to make sure all of that stays in sync. It could be problematic if NServiceBus changed any of those settings or the naming convention of the queues.

As of now, we don't have any code in our projects that uses the azure libraries directly because NServiceBus handles all the low level transport stuff for us. I still think exposing the property would make things much easier, but this could be a viable alternative if exposing the property is not an option.

SeanFeldman commented 3 years ago

It could be problematic if NServiceBus changed any of those settings or the naming convention of the queues.

The transport does not modify the queue settings if a queue exists. The queue name is always the endpoint logical name.

I still think exposing the property would make things much easier, but this could be a viable alternative if exposing the property is not an option.

I'm merely offering a workaround to ensure you're not blocked while the decision to have the transport updated with this feature or not is being made. Hope that helps.

As of now, we don't have any code in our projects that uses the azure libraries directly because NServiceBus handles all the low level transport stuff for us.

If you need to see what the code would look like, here it is: https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/blob/a950cf62ac7b424b9d7279076fdc02abe7eb2d09/src/CommandLine/Queue.cs#L10-L22