Skip to content

Conversation

MichaelSnowden
Copy link
Contributor

@MichaelSnowden MichaelSnowden commented Sep 19, 2022

This is for #3368. It adds a check to our transfer task processor which ensures that tasks are closed before deleting them.

@MichaelSnowden MichaelSnowden changed the title Make Transfer Queue Processor ensure that tasks are closed before deleting them Retry attempts to delete open workflow executions Sep 19, 2022
@MichaelSnowden MichaelSnowden force-pushed the txfer-task-executor branch 6 times, most recently from deab139 to 35fb1ce Compare September 20, 2022 19:59
@MichaelSnowden MichaelSnowden marked this pull request as ready for review September 20, 2022 20:00
@MichaelSnowden MichaelSnowden requested a review from a team as a code owner September 20, 2022 20:00
@MichaelSnowden MichaelSnowden force-pushed the txfer-task-executor branch 3 times, most recently from a37cdc0 to 636101b Compare September 21, 2022 19:12
@MichaelSnowden MichaelSnowden force-pushed the txfer-task-executor branch 2 times, most recently from dcc261f to 9753ac0 Compare September 21, 2022 21:05
@@ -273,6 +280,12 @@ func (t *transferQueueTaskExecutorBase) deleteExecution(
}
}

if checkForPendingCloseExecutionTask {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it is cleaner to move this logic into processDeleteExecutionTask and keep deleteExecution() as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, but I wanted it to be there so that I don't have to load mutable state twice.

@MichaelSnowden MichaelSnowden enabled auto-merge (squash) September 27, 2022 19:39
@MichaelSnowden MichaelSnowden merged commit e3e1cce into temporalio:master Sep 27, 2022
@MichaelSnowden MichaelSnowden deleted the txfer-task-executor branch December 29, 2022 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants