Find the issue: long requests

Spread the love

In the previous post, I discussed a few design changes to a sample Web API controller to fix up the parameters. Here is where we ended up:

public sealed class MyCommandController : ApiController
{
    public string Post([FromUri] MyCommandParameters p)
    {
        Process myCommand = Process.Start(
            "MyCommand.exe",
            $"-useTheForce {p.useTheForce} -timeout {p.timeout} -auditMessage {p.auditMessage}");
        myCommand.WaitForExit();
        return $"Completed with exit code {myCommand.ExitCode}";
    }
}

Of course, we’re not done yet. I ask again: can you find the next issue in this ASP.NET Web API code snippet?

Today I would like to direct your attention to that WaitForExit call. This is a blocking call which unnecessarily ties up the request thread. You might think, “No problem, let’s just use an async controller action instead!”

Unfortunately, you will immediately run into another problem once you go that route: there is no async version of WaitForExit. All is not lost, however; you could use the Exited event to cobble together an async Process pattern. It would be a bit of work but certainly achievable. Still, you would have a more fundamental problem — how long are you actually willing to tie up the HTTP request waiting for the process to exit?

In the general case, it is hard to predict how long an external action may take to complete. Even if usually takes just a few seconds to execute the command, there will always be outliers (as Mark Russinovich can attest). We can also use the clue that the command itself in this example takes in a timeout parameter measured in seconds. It is safe to say that the request here would commonly need to wait quite a while, relatively speaking.

Not to worry, though. This is what HTTP 202 was made for:

The 202 response is intentionally noncommittal. Its purpose is to allow a server to accept a request for some other process […] without requiring that the user agent’s connection to the server persist until the process is completed.

So what we would want to do here is return an initial 202 response with some basic status tracking information and allow the user to call back later for the result. This same technique is used by the Azure Resource Manager for its asynchronous REST API.

For this example, we will use a design where all the work is delegated to a MyCommands class. It will start new commands with a Start method and track existing commands with a Get method. The controller actions will be modified accordingly; the Post method will call Start and return a tracking ID right away, while the new Get method will return the status based on ID. The basic skeleton now looks like this:

public sealed class MyCommandController : ApiController
{
    private readonly MyCommands commands;

    public MyCommandController(MyCommands commands)
    {
        this.commands = commands;
    }

    public HttpResponseMessage Post([FromUri] MyCommandParameters p)
    {
        return this.Request.CreateResponse(HttpStatusCode.Accepted, this.commands.Start(p));
    }

    public MyCommandResult Get(string id)
    {
        return this.commands.Get(id);
    }
}

Note the use of a new data type MyCommandResult to wrap the original command completion message. This is so we can allow programmatic detection of the pending/finished state to aid in client polling.

To make the routes look nice, we would want the experience of calling POST on api/MyCommand/ and GET on api/MyCommand/id. This is relatively easy in Web API by using convention-based routing:

            config.Routes.MapHttpRoute(
                name: "DefaultApi",
                routeTemplate: "api/{controller}/{id}",
                defaults: new { id = RouteParameter.Optional });

Let’s create the MyCommands class next. This is where all the Process functionality will now live:

public sealed class MyCommands
{
    private readonly ConcurrentDictionary<string, Process> commands;

    public MyCommands()
    {
        this.commands = new ConcurrentDictionary<string, Process>();
    }

    public string Start(MyCommandParameters p)
    {
        Process myCommand = Process.Start(
            "MyCommand.exe",
            $"-useTheForce {p.useTheForce} -timeout {p.timeout} -auditMessage {p.auditMessage}");
        string id = Guid.NewGuid().ToString();
        this.commands[id] = command;
        return id;
    }

    public MyCommandResult Get(string id)
    {
        Process command;
        if (!this.commands.TryGetValue(id, out command))
        {
            return MyCommandResult.NotFound(id);
        }

        if (!command.HasExited)
        {
            return MyCommandResult.Pending(id);
        }

        return MyCommandResult.Completed(id, command.ExitCode);
    }
}

We are using a ConcurrentDictionary to store the ID to Process mapping, since many parallel requests could come in and we do not want to corrupt our state. The results are generated using MyCommandResult via the named constructor idiom:

public sealed class MyCommandResult
{
    private MyCommandResult(string id, MyCommandStatus status, string detail)
    {
        this.Id = id;
        this.Status = status;
        this.Detail = detail;
    }

    public string Id { get; }

    public MyCommandStatus Status { get; }

    public string Detail { get; }

    public static MyCommandResult NotFound(string id)
    {
        return new MyCommandResult(id, MyCommandStatus.NotFound, $"The command {id} was not found.");
    }

    public static MyCommandResult Pending(string id)
    {
        return new MyCommandResult(id, MyCommandStatus.Pending, $"The command {id} is still running.");
    }

    public static MyCommandResult Completed(string id, int exitCode)
    {
        return new MyCommandResult(id, MyCommandStatus.Completed, $"The command {id} completed with exit code {exitCode}.");
    }
}

Finally, the simple status enum for this three-value model:

public enum MyCommandStatus
{
    NotFound,
    Pending,
    Completed
}

Admittedly, we have added dozens more lines of code using this approach. But the payoff is improved concurrency, efficiency, and flexibility. In any case, the extra code is certainly not complex — one might even say it is simple to read and write.

So, are we done? Not yet, I’m afraid. Since our controller no longer has a public parameterless constructor, the default resolver will not be able to instantiate it for us anymore. Rather than go to a full blown DI container like Unity, I generally prefer to hand-code a simple HTTP controller activator such as in Mark Seemann’s example. For this situation, the following will suffice:

using System;
using System.Net.Http;
using System.Web.Http.Controllers;
using System.Web.Http.Dispatcher;

internal sealed class MyActivator : IHttpControllerActivator
{
    private readonly MyCommands commands;

    public MyActivator(MyCommands commands)
    {
        this.commands = commands;
    }

    public IHttpController Create(HttpRequestMessage request, HttpControllerDescriptor controllerDescriptor, Type controllerType)
    {
        if (controllerType == typeof(MyCommandController))
        {
            return new MyCommandController(this.commands);
        }

        return null;
    }
}

To register the activator, you need to add one more line to your Startup.Configuration
method:

config.Services.Replace(typeof(IHttpControllerActivator), new MyActivator(new MyCommands()));

Now we have a fully functional controller with proper async operation support! If you POST, you should quickly get back a response like so:

HTTP/1.1 202 Accepted
Content-Length: 38
Content-Type: application/json; charset=utf-8
Date: Sat, 29 Sep 2018 17:52:03 GMT
Server: Microsoft-HTTPAPI/2.0

"dea4ce75-baed-4232-b6be-cc076aceef55"

Given the ID, you can GET the status of the operation:

// When still running...
{
  "Id": "dea4ce75-baed-4232-b6be-cc076aceef55",
  "Status": 1,
  "Detail": "The command dea4ce75-baed-4232-b6be-cc076aceef55 is still running."
}

// When completed...
{
  "Id": "dea4ce75-baed-4232-b6be-cc076aceef55",
  "Status": 2,
  "Detail": "The command dea4ce75-baed-4232-b6be-cc076aceef55 completed with exit code 0."
}

// When not found...
{
  "Id": "abcd",
  "Status": 0,
  "Detail": "The command abcd was not found."
}

It functions properly, and yet there are still a few issues here. But we will dig into those next time….

One thought on “Find the issue: long requests

  1. Pingback: Find the issue: unbounded growth – WriteAsync .NET

Leave a Reply

Your email address will not be published. Required fields are marked *

Time limit is exhausted. Please reload the CAPTCHA.