a

    Alex Cano

    2 years ago
    @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?
    Jeremiah

    Jeremiah

    2 years ago
    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

    2 years ago
    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!
    Jeremiah

    Jeremiah

    2 years ago
    I think that’s a good callout - as we mature the feature we can continue to reduce that potential