It's better to only change the workflow instance host in TaskExecutionContext, and inject ITaskExecutorRepository into TaskExecutorLifecycleEventRemoteReporter.

We can remove workflowInstanceHost from IReportableTaskExecutorLifecycleEvent, as this event does not inherently require knowledge of the workflow instance host. Instead, when reporting the event, the reporter should retrieve the current host from a centralized repository. This approach simplifies workflow host management and helps prevent potential concurrency issues. Once the workflow host changed, the reporter can get the correct host when report a event.

Or we can set the workflow instance host into ReportableTaskExecutorLifecycleEventChannel, then we need to register the channel into reporter when the TaskExecutor registered, and change the host in channel when reassignWorkflowInstanceHost

ok.

get host address from TaskExecutor may not work, TaskExecutor's lifecycle and event's lifecycle are not completely overlapping, TaskExecutor will removed in onTaskExecutorFinalizeLifecycleEvent before the all events be reported.

but the channel‘s lifecycle is similar to event, it still needs to lock.

TaskExecutorFinalizeLifecycleEvent is the end event, and it doesn't rely on RPC, it's only a local event(It's used to do some resource clean work), so it's ok to assembly the host to channel?

yes, i mean, i did test for get host address from task-executor, when reporter handle the failed or other final events, the task-executor is already removed from repository.

assembly the host to channel is not ok, channel will removed when current events are all reported, at this moment reassign host will not work, this is a lifecycle overlapping issue, Channel or task-executor must have one to cover the task's all lifecycle to hold the host.

i'm thinking to keep the channel's lifecycle when it's empty, and inject ITaskExecutorRepository into TaskExecutorLifecycleEventRemoteReporter use to check the task-executor exists, if channel is empty and task-executor was removed, then remove the channel.

I think it because reporter and coordinator are in different thread, they are async to handle the event, so the task executor might be removed prematurely.

A TaskExecutor will only be removed after the TaskExecutorFinalizeLifecycleEvent is fired, and this event should be fired in synchronous mode.

A TaskExecutor will only be removed after the TaskExecutorFinalizeLifecycleEvent is fired, and this event should be fired in synchronous mode.

so, how about moving the publishing of TaskExecutorFinalizeLifecycleEvent to TaskExecutorLifecycleEventRemoteReporter? When a task reaches a terminal state (Success/Killed/Paused/Failed), we could publish a new TaskExecutorFinishedLifecycleEvent to mark the TaskExecutor as invalid or inactive. Then, the final event TaskExecutorFinalizeLifecycleEvent would instead be published by TaskExecutorLifecycleEventRemoteReporter.

I've submitted the new changes implemented according to this flow.

Good idea. After the TaskExecutor received the ACK of a terminal event, then means the task executor's lifecycle is finished, then publish a Finalize event to clean the resource.

submitted.

Please don't import a new Event type here. We don't really need this.

resubmitted.

This is too complex. It's better to publish TaskExecutorFinalizeLifecycleEvent in receiveTaskExecutorLifecycleEventACK after receive a finished ack.

wow... it's pretty much better!

resubmitted.

It's better to revert this method, and change the implementation. Once the workflow host changed, we may need to reset the event's lastReportTime or set a new flag to mark the event need to retry now, and notify the reporter by taskExecutionEventEmptyCondition.signalAll().

resubmitted.

apply committed