Testing races with a slow Decorator

Delaying database interactions for test purposes.

In chapter 12 in Code That Fits in Your Head, I cover a typical race condition and how to test for it. The book comes with a pedagogical explanation of the problem, including a diagram in the style of Designing Data-Intensive Applications. In short, the problem occurs when two or more clients are competing for the last remaining seats in a particular time slot.

In my two-day workshop based on the book, I also cover this scenario. The goal is to show how to write automated tests for this kind of non-deterministic behaviour. In the book, and in the workshop, my approach is to rely on the law of large numbers. An automated test attempts to trigger the race condition by trying 'enough' times. A timeout on the test assumes that if the test does not trigger the condition in the allotted time window, then the bug is addressed.

At one of my workshops, one participant told me of a more efficient and elegant way to test for this. I wish I could remember exactly at which workshop it was, and who the gentleman was, but alas, it escapes me.

Reproducing the condition #

How do you deterministically reproduce non-deterministic behaviour? The default answer is almost tautological. You can't, since it's non-deterministic.

The irony, however, is that in the workshop, I deterministically demonstrate the problem. The problem, in short, is that in order to decide whether or not to accept a reservation request, the system first reads data from its database, runs a fairly complex piece of decision logic, and finally writes the reservation to the database - if it decides to accept it, based on what it read. When competing processes vie for the last remaining seats, a race may occur where both (or all) base their decision on the same data, so they all come to the conclusion that they still have enough remaining capacity. Again, refer to the book, and its accompanying code base, for the details.

How do I demonstrate this condition in the workshop? I go into the Controller code and insert a temporary, human-scale delay after reading from the database, but before making the decision:

var reservations = await Repository.ReadReservations(r.At);
 
await Task.Delay(TimeSpan.FromSeconds(10));
 
if (!MaitreD.WillAccept(DateTime.Now, reservations, r))
    return NoTables500InternalServerError();

await Repository.Create(restaurant.Id, reservation);

Then I open two windows, from which I, within a couple of seconds of each other, try to make competing reservations. When the bug is present, both reservations are accepted, although, according to business rules, only one should be.

So that's how to deterministically demonstrate the problem. Just insert a long enough delay.

We can't, however, leave such delays in the production code, so I never even considered that this simple technique could be used for automated testing.

Slowing things down with a Decorator #

That's until my workshop participant told me his trick: Why don't you slow down the database interactions for test-purposes only? At first, I thought he had in mind some nasty compiler pragmas or environment hacks, but no. Why don't you use a Decorator to slow things down?

Indeed, why not?

Fortunately, all database interaction already takes place behind an IReservationsRepository interface. Adding a test-only, delaying Decorator is straightforward.

public sealed class SlowReservationsRepository : IReservationsRepository
{
    private readonly TimeSpan halfDelay;
 
    public SlowReservationsRepository(
        TimeSpan delay,
        IReservationsRepository inner)
    {
        Delay = delay;
        halfDelay = delay / 2;
        Inner = inner;
    }
 
    public TimeSpan Delay { get; }
    public IReservationsRepository Inner { get; }
 
    public async Task Create(int restaurantId, Reservation reservation)
    {
        await Task.Delay(halfDelay);
        await Inner.Create(restaurantId, reservation);
        await Task.Delay(halfDelay);
    }
 
    public async Task Delete(int restaurantId, Guid id)
    {
        await Task.Delay(halfDelay);
        await Inner.Delete(restaurantId, id);
        await Task.Delay(halfDelay);
    }
 
    public async Task<Reservation?> ReadReservation(
        int restaurantId,
        Guid id)
    {
        await Task.Delay(halfDelay);
        var result = await Inner.ReadReservation(restaurantId, id);
        await Task.Delay(halfDelay);
        return result;
    }
 
    public async Task<IReadOnlyCollection<Reservation>> ReadReservations(
        int restaurantId,
        DateTime min,
        DateTime max)
    {
        await Task.Delay(halfDelay);
        var result = await Inner.ReadReservations(restaurantId, min, max);
        await Task.Delay(halfDelay);
        return result;
    }
 
    public async Task Update(int restaurantId, Reservation reservation)
    {
        await Task.Delay(halfDelay);
        await Inner.Update(restaurantId, reservation);
        await Task.Delay(halfDelay);
    }
}

This one uniformly slows down all operations. I arbitrarily decided to split the Delay in half, in order to apply half of it before each action, and the other half after. Honestly, I didn't mull this over too much; I just thought that if I did it that way, I wouldn't have to speculate whether it would make a difference if the delay happened before or after the action in question.

Slowing down tests #

I added a few helper methods to the RestaurantService class that inherits from WebApplicationFactory<Startup>, mainly to enable decoration of the injected Repository. With those changes, I could now rewrite my test like this:

[Fact]
public async Task NoOverbookingRace()
{
    var date = DateTime.Now.Date.AddDays(1).AddHours(18.5);
    using var service = RestaurantService.CreateWith(repo =>
        new SlowReservationsRepository(
            TimeSpan.FromMilliseconds(100), repo));
 
    var task1 = service.PostReservation(new ReservationDtoBuilder()
        .WithDate(date)
        .WithQuantity(10)
        .Build());
    var task2 = service.PostReservation(new ReservationDtoBuilder()
        .WithDate(date)
        .WithQuantity(10)
        .Build());
    var actual = await Task.WhenAll(task1, task2);
 
    Assert.Single(
        actual,
        msg => msg.StatusCode == HttpStatusCode.InternalServerError);
    var ok = Assert.Single(actual, msg => msg.IsSuccessStatusCode);
    // Check that the reservation was actually created:
    var resp = await service.GetReservation(ok.Headers.Location);
    resp.EnsureSuccessStatusCode();
    var reservation = await resp.ParseJsonContent<ReservationDto>();
    Assert.Equal(10, reservation.Quantity);
}

The restaurant being tested has a maximum capacity of ten guests, so while it can accommodate either of the two requests, it can't make room for both.

For this example, I arbitrarily chose to configure the Decorator with a 100-millisecond delay. Every interaction with the database caused by that test gets a built-in 100-millisecond delay. 50 ms before each action, and 50 ms after.

The test starts both tasks, task1 and task2, without awaiting them. This allows them to run concurrently. After starting both tasks, the test awaits both of them with Task.WhenAll.

The assertion phase of the test is more involved than you may be used to see. The reason is that it deals with more than one possible failure scenario.

The first two assertions (Assert.Single) deal with the complete absence of transaction control in the application. In that case, both POST requests succeed, which they aren't supposed to. If the system works properly, it should accept one request and reject the other.

The rest of the assertions check that the successful reservation was actually created. That's another failure scenario.

The way I chose to deal with the race condition is standard in .NET. I used a TransactionScope. This is peculiar and, in my opinion, questionable API that enables you to start a transaction anywhere in your code, and then complete when you you're done. In the code base that accompanies Code That Fits in Your Head, it looks like this:

private async Task<ActionResult> TryCreate(Restaurant restaurant, Reservation reservation)
{
    using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled);
 
    var reservations = await Repository
        .ReadReservations(restaurant.Id, reservation.At)
        .ConfigureAwait(false);
    var now = Clock.GetCurrentDateTime();
    if (!restaurant.MaitreD.WillAccept(now, reservations, reservation))
        return NoTables500InternalServerError();
 
    await Repository.Create(restaurant.Id, reservation).ConfigureAwait(false);
 
    scope.Complete();
 
    return Reservation201Created(restaurant.Id, reservation);
}

Notice the scope.Complete() statement towards the end.

What happens if someone forgets to call scope.Complete()?

In that case, the thread that wins the race returns 201 Created, but when the scope goes out of scope, it's disposed of. If Complete() wasn't called, the transaction is rolled back, but the HTTP response code remains 201. Thus, the two assertions that inspect the response codes aren't enough to catch this particular kind of defect.

Instead, the test subsequently queries the System Under Test to verify that the resource was, indeed, created.

Wait time #

The original test shown in the book times out after 30 seconds if it can't produce the race condition. Compared to that, the refactored test shown here is fast. Even so, we may fear that it spends too much time doing nothing. How much time might that be?

The TryCreate helper method shown above is the only part of a POST request that interacts with the Repository. As you can see, it calls it twice: Once to read, and once to write, if it decides to do that. With a 100 ms delay, that's 200 ms.

And while the test issues two POST requests, they run in parallel. That's the whole point. It means that they'll still run in approximately 200 ms.

The test then issues a GET request to verify that the resource was created. That triggers another database read, which takes another 100 ms.

That's 300 ms in all. Given that these tests are part of a second-level test suite, and not your default developer test suite, that may be good enough.

Still, that's the POST scenario. I also wrote a test that checks for a race condition when doing PUT requests, and it performs more work.

[Fact]
public async Task NoOverbookingPutRace()
{
    var date = DateTime.Now.Date.AddDays(1).AddHours(18.5);
    using var service = RestaurantService.CreateWith(repo =>
        new SlowReservationsRepository(
            TimeSpan.FromMilliseconds(100), repo));
    var (address1, dto1) = await service.PostReservation(date, 4);
    var (address2, dto2) = await service.PostReservation(date, 4);
 
    dto1.Quantity += 2;
    dto2.Quantity += 2;
    var task1 = service.PutReservation(address1, dto1);
    var task2 = service.PutReservation(address2, dto2);
    var actual = await Task.WhenAll(task1, task2);
 
    Assert.Single(
        actual,
        msg => msg.StatusCode == HttpStatusCode.InternalServerError);
    var ok = Assert.Single(actual, msg => msg.IsSuccessStatusCode);
    // Check that the reservations now have consistent values:
    var client = service.CreateClient();
    var resp1 = await client.GetAsync(address1);
    var resp2 = await client.GetAsync(address2);
    resp1.EnsureSuccessStatusCode();
    resp2.EnsureSuccessStatusCode();
    var body1 = await resp1.ParseJsonContent<ReservationDto>();
    var body2 = await resp2.ParseJsonContent<ReservationDto>();
    Assert.Single(new[] { body1.Quantity, body2.Quantity }, 6);
    Assert.Single(new[] { body1.Quantity, body2.Quantity }, 4);
}