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

Handle data loss error in replication read history #3704

Merged
merged 5 commits into from Dec 13, 2022

Conversation

yux0
Copy link
Contributor

@yux0 yux0 commented Dec 12, 2022

What changed?

  1. Use data loss error for corrupted data case.
  2. Ability to bypass corrupted data in replication.

Why?
We should aware of the corrupted data but also need to ability to bypass this error so it will not block the replication.
This is a clean version of #3691

How did you test it?
local test.

Potential risks

Is hotfix candidate?
No.

@yux0 yux0 requested a review from a team as a code owner December 12, 2022 18:32
service/history/replication/ack_manager.go Show resolved Hide resolved
service/history/replication/ack_manager.go Outdated Show resolved Hide resolved
err error,
) error {
switch err.(type) {
case *serviceerror.NotFound, *serviceerror.DataLoss:
Copy link
Member

Choose a reason for hiding this comment

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

Why NotFound is here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the first batch of the history is lost, it will return not found.

return nil, nil, nil, 0, serviceerror.NewNotFound("Workflow execution history not found.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One assumption here is history will be there as long as mutable state is here.

@yux0 yux0 merged commit 8ca4b2c into temporalio:master Dec 13, 2022
@yux0 yux0 deleted the replication-error branch December 13, 2022 00:04
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.

None yet

2 participants