Or, read, comprehend, then post.
So we had a minor outage tonight at Honeycomb, and the thing that took the longest to recover was a backlog in Service Level Objective processing.
I pulled open the profiles to figure out why the lagging 3 partitions had gotten so stuck, and the answer was that they were thrashing running ctx, _ := context.WithTimeout(ctx, time.Minute) before running DatabaseFetch(ctx). So my first thought was "why don't we make it faster? Surely it doesn't have to timeout in exactly 60 seconds?" and then I went down a whole big rabbit hole adapting an existing library we had for approximate select {} channel fallbacks/timeouts to cancel the database call so we wouldn't construct an exact time for the timeout.
And it worked, but it was gnarly and messy and I had no confidence it would actually function in prod, whether there were guarantees of thread safety, and so on. And then I read the full code of DatabaseFetch(). And it read: if value, ok := cache[key]; ok {. Whoops. I could have moved the timeout into only the cache miss case much earlier, but I'd prematurely optimised the timeout above based on a function name without fully comprehending that no, of course we weren't accessing the database on each record, that would be silly.
Calling expensive functions less frequently has a lot more bang than making expensive functions run faster. The pull request fixing the performance bug only required moving 3 lines of code within a file, after an hour spent gnashing my teeth and writing several dozen lines of baroque code that I wound up throwing away.
