Skip to content

Fix race when storing LogStream resource names

Adam Coldrick requested to merge sotk/tech-debt/logstream-race into master

Description

I found this fun race condition where most of the time the caller of Execute doesn't get to know about the LogStream resource names until the final message in the response stream. This is because we store the LogStream names after we update the Job's stage from QUEUED to EXECUTING, in a separate transaction. That stage transition triggers the penultimate Operation message, and if the Execution service picks up this transition before the Bots service has finished storing the LogStream names then they won't be populated in this message, leaving the client stuck without the names until the Job is completed anyway.

This MR fixes this race by moving the stage transition to after the LogStream names are stored. Also in this MR is a small tidy-up removing a couple of methods from the Job class which are no longer necessary for storing the LogStream names now that there's no in-memory datastore to worry about.

A more ideal fix would be to do all of these updates whilst handling Lease acceptance in the same transaction, but that requires !971 (merged) and the intended further refactoring around our session handling.

Edited by Adam Coldrick

Merge request reports