GaneshPatil7517 commented on issue #17751:
URL: 
https://github.com/apache/dolphinscheduler/issues/17751#issuecomment-3858314411

   Hi @ruanwenjun, thank you for the feedback. I apologize for not providing a 
design document upfront.
   
   Here's my understanding of the architecture and the proposed changes:
   
   Problem
   Currently, WorkflowExecutionGraph is created inside 
[handleCommand()](vscode-file://vscode-app/c:/Users/HP/AppData/Local/Programs/Microsoft%20VS%20Code/bdd88df003/resources/app/out/vs/code/electron-browser/workbench/workbench.html)
 which runs within a database transaction. This increases transaction time and 
lock contention.
   
   **Current Flow**
   handleCommand() [@Transactional]
     → assembleWorkflowInstance()
     → assembleWorkflowGraph()
     → createWorkflowExecutionGraphAssembler() ← Graph created HERE (inside tx)
     → triggerWorkflow()
   
   **Proposed Flow**
   handleCommand() [@Transactional]
     → assembleWorkflowInstance()
     → assembleWorkflowGraph()
     → triggerWorkflow() (no graph yet)
     
   WorkflowRunningStateAction.onStartEvent() [OUTSIDE tx]
     → WorkflowExecutionGraphFactory.createGraph() ← Graph created HERE
     → context.setWorkflowExecutionGraph(graph)
   
   **Key Changes**
   WorkflowExecutionGraphFactory - Centralizes graph creation for all command 
types (START_PROCESS, REPEAT_RUNNING, START_FAILURE_TASK_PROCESS, 
RECOVER_TOLERANCE_FAULT_PROCESS, RECOVER_SERIAL_WAIT)
   Removed assembler pattern - Per your earlier feedback that "command handlers 
should not be concerned with how instances are initialized"
   Deferred initialization - Graph is created when WorkflowStartLifecycleEvent 
fires, after the transaction
   Please let me know if you'd like me to elaborate on any part of the design 
or if this approach needs adjustment.
   
   Would you like me to modify the design document or create a different format 
for posting on the PR?> handleCommand() [@Transactional]
   
   → assembleWorkflowInstance()
   → assembleWorkflowGraph()
   → createWorkflowExecutionGraphAssembler() ← Graph created HERE (inside tx)
   → triggerWorkflow()
   
   
   ## Proposed Flow
   handleCommand() [@Transactional]
   → assembleWorkflowInstance()
   → assembleWorkflowGraph()
   → triggerWorkflow() (no graph yet)
   
   WorkflowRunningStateAction.onStartEvent() [OUTSIDE tx]
   → WorkflowExecutionGraphFactory.createGraph() ← Graph created HERE
   → context.setWorkflowExecutionGraph(graph)
   
   
   ## Key Changes
   1. **WorkflowExecutionGraphFactory** - Centralizes graph creation for all 
command types (START_PROCESS, REPEAT_RUNNING, START_FAILURE_TASK_PROCESS, 
RECOVER_TOLERANCE_FAULT_PROCESS, RECOVER_SERIAL_WAIT)
   2. **Removed assembler pattern** - Per your earlier feedback that "command 
handlers should not be concerned with how instances are initialized"
   3. **Deferred initialization** - Graph is created when 
WorkflowStartLifecycleEvent fires, after the transaction
   
   Please let me know if you'd like me to elaborate on any part of the design 
or if this approach needs adjustment.
   
   
[DESIGN-17751.md](https://github.com/user-attachments/files/25120043/DESIGN-17751.md)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to