-
Notifications
You must be signed in to change notification settings - Fork 874
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
Task channel aware rescheduling #3266
Conversation
rescheduled.rescheduleTime.Add(rescheduleFailureBackoff) | ||
failToSubmit = append(failToSubmit, rescheduled) | ||
if pq.IsEmpty() { | ||
delete(r.pqMap, key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe worth to consider object pooling if profiling shows frequent create/destroy pq become a bottleneck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah make sense.
I am also thinking that maybe this check can be moved to the beginning of the loop, so we don't immediately clean up the pq when it's empty but wait for the next round to see if it's still empty. If there's no next round, storing some empty pq seems ok to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the logic to clean up empty PQ every 3 minutes to avoid keep creating & deleting pq, especially for scheduled queue.
I am going to land this PR. Let me know if you have any concern re. ^ change. If so, I am happy to change it in a later pr. Thanks.
What changed?
Why?
How did you test it?
Potential risks
Is hotfix candidate?