Backward Compatibility: Parallel Branch does not support explicit control flow anymore

[yzhao244]

In our spec, currently, it does not support end-user to explicitly define states in branch which was supported in 0.1

I think nowadays workflow products provide more and more user-friendly UI to quickly/efficiently build workflows. Our spec currently requires that end-user has to create subFlow first and then gets a workflowId. Finally, end-user has to put that workflowId in the branches. My personal feeling is a bit complex process which makes UI render graphical model a bit more complicated comparing to directly allowing user to define explicit control flow.

Therefore, I would like to propose supporting both actions and explicitly defining states in branch.

Parameter	Description	Type	Required
name	Branch name	string	yes
actions	Actions to be executed in this branch	array	no
maxLayers	Limits to max number of nested layers. Required, if explicit states are defined	integer	no
states	States to be executed in this branch	array	no
timeouts	Branch specific timeout settings	object	no

[tsurdilo]

Here are the reasons on why this ability was previously removed from the spec. This is not to say that we don't want to add this feature again, but just to document the issues:

1. Rules that come with ability to add states to branches are complex to define and verify. You have to assure that no states defined in the branches can transition to any top-workflow states, other states in branches for example.
2. It is very inconvenient for large workflows with many branches: Sub-workflows provide reusability and better maintenance than having to write and maintain large workflows with multiple states inside hundreds of branches.
3. Harder to debug: self-explanatory for large workflows
4. Additional rules that have to apply: branch states cannot use certain aspects such as termination end definition, timeouts, overwrite things that are set on the parallel state level that includes them. In addition when defined via sub-workflows those can use continueAs without problems, where that would also have to be prohibited for use if states were in-lined into branches.

Let's start from here and try to figure out how to incorporate this into spec.

[tsurdilo]

@yzhao244 @cdavernas @ricardozanini @antmendoza

Here is a proposal:

1. Current:

   {
   "id": "parallelexec",
   "version": "1.0",
   "specVersion": "0.7",
   "name": "Parallel Execution Workflow",
   "description": "Executes two branches in parallel",
   "start": "ParallelExec",
   "states":[
   {
   "name": "ParallelExec",
   "type": "parallel",
   "completionType": "allOf",
   "branches": [
   {
   "name": "ShortDelayBranch",
   "actions": [{
   "subFlowRef": "shortdelayworkflowid"
   }]
   },
   {
   "name": "LongDelayBranch",
   "actions": [{
   "subFlowRef": "longdelayworkflowid"
   }]
   }
   ],
   "end": true
   }
   ]
   }

2. Proposed change (inline state defs in branches):

   {
   "id": "parallelexec",
   "version": "1.0",
   "specVersion": "0.7",
   "name": "Parallel Execution Workflow",
   "description": "Executes two branches in parallel",
   "start": "ParallelExec",
   "states": [
   {
   "name": "ParallelExec",
   "type": "parallel",
   "completionType": "allOf",
   "branches": [
   {
   "name": "ShortDelayBranch",
   "branchStates": "shortDelayBranchStates"
   },
   {
   "name": "LongDelayBranch",
   "branchStates": "longDelayBranchStates"
   }
   ],
   "end": true
   }
   ],
   "branchstates": [
   {
   "name": "shortDelayBranchStates",
   "start": "shorDelay",
   "states": [
   {
   "name": "shorDelay",
   "type": "operation",
   "actions": [
   {
   "functionRef": "callShortDelayFunction"
   }
   ],
   "end": true
   }
   ]
   },
   {
   "name": "longDelayBranchStates",
   "start": "longDelay",
   "states": [
   {
   "name": "longDelay",
   "type": "operation",
   "actions": [
   {
   "functionRef": "callLongDelayFunction"
   }
   ],
   "end": true
   }
   ]
   }
   ]
   }

[cdavernas]

@tsurdilo Personally not a big fan of the proposal. It adds a extreme level of complexity for runtimes, infinite nesting possibility (even with the prop to limit those), etc. Plus, it will be near impossible to comprehend/control such flows, should it be visually or in a pure data perspective. To give you an idea, imagine a ParralelExec with states that contain ParralelExecs, with substates that contain ParralelExecs. To me, it's an absolute model smell, and I doubt any runtime will ever be able to run that properly with all existing features.
Though I get that SW is a DSL, we should IMO keep in mind that it becomes utterly useless if no runtime can effectively run it, and should therefore think of the implications for such implementations

[tsurdilo]

@cdavernas yeah its just a first attempt. the proposal does not solve issues that are mentioned in this comment: #466 (comment)

I think we all agree that this makes life somewhat easier for end user but it does add a number of implications to runtimes as well as the spec having to add a lot of rules around it that are hard to impl and understand.

Maybe we can find some middle ground at some point, so let's keep discussing.

[yzhao244]

@cdavernas @tsurdilo maybe adding "maxLayer" could help on resolving the case of adding complexity of runtime. I agree that some complexity is added to runtime implementation. However, I think the concept of supporting branch explicit control is being supported by AWS DSL and step function runtime engine along with other dsl and their runtime engines. My feeling is that its just an attempt to adding a missing feature and make life of end user easier. :)

[tsurdilo]

@cdavernas @yzhao244
what would maxLayer do in this case?
i still think that state groups would be best to allow this feature.
@yzhao244 could you show me a sample of how aws has this? Do they have any rules on dealing with issues that come with embedding states in branches? Would be great to see.

[yzhao244]

@tsurdilo Sure, a quick highlight of branch definition difference against aws dsl which is branch states vs branch actions

The following is an example of how aws does for embedding states in branches. (https://states-language.net/spec.html)

The rule on dealing with embedding states in branches as following quote from aws dsl:
"A state in a Parallel State branch "States" field MUST NOT have a "Next" field that targets a field outside of that "States" field. A state MUST NOT have a "Next" field which matches a state name inside a Parallel State branch’s "States" field unless it is also inside the same "States" field.

Put another way, states in a branch’s "States" field can transition only to each other, and no state outside of that "States" field can transition into it."

[tsurdilo]

Thanks! Yeah thats pretty much how we had it as well before making the change.
On our end we can apply the same rules (just would have more of them).

I think overall there is a reason why the samples they show only include a single state :) If they showed what branches with like 10+ states would look like it would be a different game :)

@cdavernas I know you are not a fan of this approach, but let's try to give it a shot. From the runtime perspective I think the problem is that given the rules of branch states as well as the ability for users to make mistakes really easily with inline states can become too complex. Just confirming if that's the case.

[tsurdilo]

@yzhao244 @cdavernas bpmn deals with this via subprocesses, which yes, each branch can have an embedded subprocess with many "tasks" / states inside, so its possible to do there. The idea of state groups is very similar where a state group cannot transition to another state group, allowing us to do whats asked for here in a similar way of bpmn for example.

[yzhao244]

@tsurdilo totally agree. I like the idea of state groups as you proposed in this discussion :)