Skip to content

Handle duplicate err in apply snapshot#9796

Open
yux0 wants to merge 3 commits intotemporalio:mainfrom
yux0:yx/new-run-rd-replication
Open

Handle duplicate err in apply snapshot#9796
yux0 wants to merge 3 commits intotemporalio:mainfrom
yux0:yx/new-run-rd-replication

Conversation

@yux0
Copy link
Copy Markdown
Contributor

@yux0 yux0 commented Apr 2, 2026

What changed?

Handle duplicate err in apply snapshot

Why?

We cannot simply return duplication if the mutable state is null. We need to reload the mutable state and retry it.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

Minor risk. This will retry one more to reload the mutable state.

@yux0 yux0 requested a review from michaely520 April 2, 2026 23:05
@yux0 yux0 requested a review from a team as a code owner April 2, 2026 23:05
@yux0 yux0 added the teams/cgs label Apr 2, 2026
return err
}
if currentRunID == targetRunID {
// Need to reload the mutable state to verify the duplication
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: does this comment really make sense here? this codepath has several callers right

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will remove this comment

Comment thread service/history/ndc/workflow_state_replicator.go
}
)

func NewSequentialTaskQueue(task TrackableExecutableTask) ctasks.SequentialTaskQueue[TrackableExecutableTask] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deadcode?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes. I don't see anywhere use it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants