1 comment

[ 4.3 ms ] story [ 10.8 ms ] thread
This is a preliminary report on the incident that grounded pretty much all planes in the UK for the best part of a day last month. I absolutely, positively love the trigger for the entire thing:

> Although there has been work by ICAO and other bodies to eradicate non-unique waypoint names there are duplicates around the world.

I mean, this is like seeing the shark fin in Jaws: even ignoring the suspenseful music, you just know there is going to be trouble...

Because, it's just perfect, right? A duplicate key, that does usually not occur, so you will happily process your entire triple-ISO-certified test suite, then a million live flight plans, until one day, someone files a flight plan that just happens to include a random collision of the same name for different waypoints on two opposite ends of the world (as an aside, I would love to see the meetings about that: you change your waypoint name! No, this waypoint name is part of our ancient traditions, you change it!).

But once that duplicate key is in the system, it goes down hard and keeps doing the exact same thing on restarts, because, well, have to process all the data! The report's conclusion about that whole thing is a masterpiece of British understatement:

> Clearly a better way to handle this specific logic error would be for FPRSA-R to identify and remove the message and avoid a critical exception.

But anyway, without further ado, here's how you can avoid having similarly embarrassing 18-page PDFs being dedicated to you -- I call it the blast-door pattern, and it works whenever there is a concept of a 'work item' anywhere in your code. Trigger warning: C#, but you can probably manage:

    enum ItemState
    {
        Created,
        Pending,
        Completed,
        Cancelled,
        Failed
    }

    void ItemProcessor(ItemQueue queue, ItemProcessor processor, ItemStore store)
    {
        var item = queue.Dequeue();
        if (item?.State == ItemState.Pending)
        {
            try
            {
                var cts = new CancellationTokenSource();
                cts.CancelAfter(TimeSpan.FromMinutes(5));
                processor.Process(cts);
                item.State = ItemState.Completed;
            }
            catch (OperationCanceledException)
            {
                item.State = ItemState.Cancelled;
            }
            catch (Exception ex)
            {
                item.Exception = ex;
                item.State = ItemState.Failed;
            }
        }
        if (item != null) store.Add(item);
    }
Now, they key takeaways here are:

1. Your state enum has an initial value that does not cause it to be processed: that is only done once the value is explicitly set to Pending. This saves you from 'oops, the item creation did not go entirely right, so it was picked up prematurely by the processor, which then crashed';

2. Items come from a queue, go back into a different store. You monitor both queue length and the number of items with a bad state in the store; if either number goes up, you have something to look into;

3. Set a time limit on processing, which should be entirely ridiculous (so, if item processing normally takes a few milliseconds, make the timeout at least a minute);

4. Yes, you catch every exception other than that, because that's like the entire point: crashing work items should be set aside to be looked at.

You'll also want some reliable tooling to reprocess failed items as needed, and one last tip before I go: don't wire up individual failed item notifications to your JIRA or something, unless you want to add a 'and then the ticket ID field had an integer overflow' war story to your arsenal...