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

fix(worker): mitigate job's locks extensions #2970

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

manast
Copy link
Contributor

@manast manast commented Dec 14, 2024

This fix tries to resolve this issue: #2965 (comment)
And this one:
#2969

@manast manast requested review from roggervalf and fgozdz December 14, 2024 13:48
@roggervalf roggervalf force-pushed the fix/edge-case-job-lock-not-renewed-in-time branch from 108e452 to 2f5492c Compare December 17, 2024 03:05
@@ -911,6 +911,8 @@ will never work with more accuracy than 1ms. */
const failed = await handleFailed(<Error>err);
return failed;
} finally {
jobsInProgress.delete(inProgressItem);
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this line be sufficient and the other extra references can be deleted as this line will be executed after completed and failed handlers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No because there are other async calls in between, so the lock extender could run in between as well and the there will be no lock for this job.

Copy link
Collaborator

Choose a reason for hiding this comment

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

before line 864 we also need to delete that inProgressitem as well

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.

2 participants