Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset for workflows without completed tasks #665

Merged
merged 8 commits into from
Aug 14, 2020

Conversation

vitarb
Copy link
Contributor

@vitarb vitarb commented Aug 11, 2020

This PR addressed an issue described in #494 allowing users to reset workflows without completed tasks.
In case if there is no completed task reset would use next event after workflow task scheduled event as a reset point.
It allows us to perform a reset on workflows that were unable to make any progress due to worker related issues.

Note that due to change in semantics two CLI parameters responsible for reset types were renamed:

  • LastWorkflowTaskCompleted was renamed to LastWorkflowTask
  • FirstWorkflowTaskCompleted was renamed to FirstWorkflowTask

@vitarb vitarb changed the title Reset wo completed tasks Reset for workflows without completed tasks Aug 11, 2020
@vitarb vitarb requested review from mastermanu and removed request for shawnhathaway August 14, 2020 01:09
for _, e := range be.Events {
eid++
if e.GetEventId() != eid {
s.Fail(fmt.Sprintf("inconintous eventID: %v, %v", eid, e.GetEventId()))
Copy link
Member

@mastermanu mastermanu Aug 14, 2020

Choose a reason for hiding this comment

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

[Nit] non-contiguous or non-continuous. Also, it is useful to add expected: and got: to the error message

Copy link
Member

Choose a reason for hiding this comment

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

also, how could this happen?

Copy link
Member

Choose a reason for hiding this comment

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

also curious when this would actually happen given we are the ones creating this mocked out data structure in the first place? We could have just assigned the eventTime variable inline for each event as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can only happen if something is messed up in the reset logic in a way that creates gaps in event IDs. We do this check in other tests so it's there for consistency reasons.

@vitarb vitarb requested review from shawnhathaway and removed request for samarabbas August 14, 2020 02:10
@vitarb vitarb merged commit dbafe8a into temporalio:master Aug 14, 2020
@vitarb vitarb deleted the reset-wo-completed-tasks branch August 14, 2020 06:14
@shawnhathaway
Copy link
Contributor

Looks good 👍

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