A failed import stamped as done waits a day to be retried
Two small bugs in a scheduled importer: failures recorded as successes, and a lease released after the database had already gone.
We run a small scheduled importer in one of our NestJS backends. Every five minutes it picks the one city whose event data is oldest, pulls events and places from public sources, and writes them in a transaction. A city counts as due when its last import is more than a day old. Nothing about it is clever, which is why the two bugs we found in production are worth writing down.
What we saw
On the first production runs, two cities failed to resolve their map position because the public map data service answered with a 504. A third city located fine and imported its events, but its places query got the same 504.
All three cities then sat quietly for a day. Nothing was wrong with the importer's schedule. The problem was what a failure wrote down.
The timestamp meant two things
The importer had one column for each kind of data, eventsImportedAt, and the due check was a plain comparison against a day ago. On a clean run we stamped it with the current time. On a failed run, a catch block also stamped it with the current time, because without that, a city that always fails would be picked first on every tick and starve everything behind it.
That reasoning is fine, but it left us with a timestamp that answered "when did we last try" while the due check read it as "when did we last succeed". A 504 that would probably clear in minutes cost a full day.
The partial case was worse. The third city's events imported, so the transaction ended with the normal stamp, and the missing places were never marked as owed. They would have waited for the next weekly places window.
The fix: say how long to wait
We did not add a second column. The smallest change that kept the existing query was to write a stamp that is deliberately in the past, so the city becomes due again after an hour instead of a day:
private retryStamp(now: Date): Date {
return new Date(now.getTime() - DAY + RETRY_FAILED_AFTER_MS);
}
Failed runs, runs that could not locate the city, and runs where events landed but places did not now all write that stamp. The starvation protection stays, because the city still moves to the back of the queue for an hour.
This is a trade-off, not a model we would defend in a design review. A timestamp that is lying on purpose is a little confusing for whoever reads the table next. If the retry rules grow beyond "an hour for any failure", the right move is an explicit nextAttemptAt column. For one rule, a named helper and a test that pins the exact value was the honest size of the change.
The lease nobody released
The importer holds a database lease so that two instances do not import at once. The lease is a row with a holder and an expiry of fifteen minutes. On shutdown we tried to release it by setting the expiry to the epoch, so the next instance would not wait out the remainder.
It never worked. We had put the release in onApplicationShutdown, and the query failed quietly because the TypeORM connection had already been closed by then. Our catch block logged a warning that nobody was reading, and a deploy could leave a lease that blocked imports for up to fifteen minutes.
NestJS runs beforeApplicationShutdown hooks before it closes connections, and onApplicationShutdown hooks after. Moving the release to the earlier hook fixed the ordering. One more guard went in with it: if an import is still running when the shutdown signal arrives, we leave the lease alone, because releasing it would let a second instance start the same city while the first is still writing.
What we check now
- Does any column that drives scheduling record the time of success, or the time of the last attempt? If it is both, write down which one wins on failure.
- Does a failure that is probably transient wait as long as a success does?
- If a run does part of its work, can the missing part be retried without redoing the rest?
- Does cleanup that touches the database run before the framework tears the database down?
- Does releasing a lock check that the work it protects has actually stopped?
Both fixes have a test that fails on the old code: one fakes a planner that returns no position and asserts the stamp is exactly twenty three hours old, and one fakes a planner that returns events but no places and asserts the places column stays empty while the events stamp is back-dated.
0 comments