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.

Sign up for GitHub

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

Jump to bottom

Ensure queue processor cluster ack level is below failover ack level #2794

Merged
merged 2 commits into from
May 2, 2022
Merged

Ensure queue processor cluster ack level is below failover ack level #2794

merged 2 commits into from
May 2, 2022

Conversation

yycptt
Copy link
Member

@yycptt yycptt commented May 2, 2022 •
edited

What changed?

  • Ensure queue processor cluster ack level is below failover ack (current) level

Why?

  • Handle task lost issue during failover when shard movement happens before failover queue tasks are processed but after persisting new active/standby queue ack level update. Task may lost because upon shard reload, the range for the new failover queue is calculated based on the updated active/standby queue ack level.
  • Verified that when deleting tasks, the failover queue's level is taken into consideration.

How did you test it?

  • eyeballing

Potential risks

  • if failover level is never deleted, then active/standby queue processor's ack level will be stuck.

Is hotfix candidate?

  • maybe

@yycptt yycptt requested review from yux0, wxing1292 and yiminc May 2, 2022 17:56
@yycptt yycptt requested a review from a team as a code owner May 2, 2022 17:56
@yycptt yycptt merged commit 08a0226 into temporalio:master May 2, 2022
@yycptt yycptt deleted the failover-min branch May 2, 2022 21:47
yycptt added a commit that referenced this pull request May 2, 2022
@yycptt
Ensure queue processor cluster ack level is below failover ack level (#…
36f6aa1 
…2794)
yycptt added a commit that referenced this pull request May 5, 2022
@yycptt
Ensure queue processor cluster ack level is below failover ack level (#…
91b099b 
…2794)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxing1292 wxing1292 wxing1292 approved these changes

@yux0 yux0 Awaiting requested review from yux0

@yiminc yiminc Awaiting requested review from yiminc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yycptt @wxing1292