https://prefect.io logo
Title
a

Alex Cano

05/11/2020, 2:17 PM
@Jeremiah @Laura Lorenz (she/her) I wanted to touch base again on the flow concurrency PR (https://github.com/PrefectHQ/prefect/pull/2382)… specifically on where to put the concurrency check in the flow state setting pipeline. I thought of 2 points that I wanted to either make sure were non-issues from past experiences or figure out how to deal with. I’m going to attach 2 images in separate posts, one for each scenario. They attempt to show what I’m going for, but I’ve never been great at diagramming! I also wanted to make sure I understood the interaction once a flow is marked as
Submitted
by the Agent. After this, from what I can tell each flow handles the transition from
Submitted
to
Running
by means of the
CloudFlowRunner
, so each
CloudFlowRunner
is calling the
set_flow_run_states
with its own flow run, right? So maybe the conditions below don’t matter? First is since the server is written using async, if we put the concurrency check in the
api.states.set_flow_run_state
call, and an API call comes in with N flow runs that are trying to transition into a
Running
state and there’s M concurrency slots where M < N, how can we guarantee only M flows will transition into the
Running
state? Second, when we’re creating the “Run Queue” in the
api.runs.get_runs_in_queue
, we’re specifically returning flow runs in the order of first scheduled. Related to the above, if we’re submitting flow runs to a
Running
state, are we guaranteeing that state changes in the flow runs submitted in the
set_flow_runs_states
mutation are occurring in order? If there are 3 flow runs and only one concurrency slot, are we just guaranteeing that one of the flow runs in the payload will succeed? Or are we guaranteeing that the first flow run in the payload will succeed?
j

Jeremiah

05/11/2020, 2:35 PM
Great question - in practice, states aren’t set for multiple runs at the same time (they’re done individually), so the surface area for this issue is extremely small and I recommend disregarding it. There IS the possibility of a minor race condition in the strategy we discussed, in between checking for concurrency availability and entering a running state, but the potential is so small I would not suggest solving it. It can be done — we do so in Cloud, but it rapidly became the single most complicated mutation we maintained (and we actually abandoned it in favor of a different approach)
a

Alex Cano

05/11/2020, 2:38 PM
Gotcha! Okay was wanting to double check that core did only ever submit one flow run state change at the same time, so that’s great to hear. Sounds like just noting that there could be a race condition there is going to be the move!
j

Jeremiah

05/11/2020, 2:38 PM
I think that’s a good callout - as we mature the feature we can continue to reduce that potential