Request throttling with asynchronous waiting

Question

I have implemented some request throttling in my app. Its based around using the MemoryCache and expiring items according to a limit. Basically i try to retrieve a cache item, if its not there I go ahead and make the request, if it is their I check the last request and workout the next available time I can make the request. My problem lies when i have to wait to make a request.

//This is a shortened version of what im doing, but the includes the required code

public void ThrottleRequest(string webservice)
{
    if (cache.TryGet(webservice))
    {
          var timeToWait = GetTimeToWaitFromSomewhere(); 
          Wait(timeToWait);
    }
}

public async void Wait(TimeSpan timeToWait)
{
     await Task.Delay(timeToWait); //This doesnt work
     Thread.Sleep(timeToWait); //This works
}

The issues are in the wait method. If i use thread.sleep numbers match up and the requests are throttled correctly (ie 1 request per second) but i would never want to use this in a production environment though. So whats the correct way to wait asychronously here? Am i misusing the API or something?

Answer 1

The problem is that your Wait method has a return type of void - you're basically saying, "Start waiting, and I'll keep going, ignoring the wait!"

You'd normally have it as:

public async Task ThrottleRequest(string webservice)
{
    if (cache.TryGet(webservice))
    {
        var timeToWait = GetTimeToWaitFromSomewhere(); 
        await Wait(timeToWait);
    }
}

public async Task Wait(TimeSpan timeToWait)
{
     await Task.Delay(timeToWait);
}

Although at that point, your Wait method is pointless, and you'd be better to write:

public async Task ThrottleRequest(string webservice)
{
    if (cache.TryGet(webservice))
    {
        var timeToWait = GetTimeToWaitFromSomewhere(); 
        await Task.Delay(timeToWait);
    }
}

Note that I've changed your ThrottleRequest method to be async and to return Task as well. It's not clear what you were hoping that calling Wait would achieve on its own, but if you're trying to introduce asynchrony, you need to be very clear about what you're doing - and usually the method which calls an async method will also be asynchronous.

If you really just wanted to pause the currently executing thread in ThrottleRequest, then you shouldn't be using an async method at all - you'd want to synchronously delay, which is precisely what Thread.Sleep does.

I strongly suspect you don't have a totally clear idea of what your code is meant to be doing in terms of being synchronous or asynchronous at the moment. I would take a step back from the code itself and work out how you actually expect it to work (think about what you want the threading model to be, for example), then go back to the keyboard.

Also consider that after the delay has occurred, you're not checking the cache again - that means if 20 different threads each get into your method at once, they'll all end up hitting the web service, instead of only one of them doing so and the rest using the cached result.