-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat(controller): optimize memory with queue when archiving is rate-limited. Fixes #13418 #13419
Conversation
038e995
to
e06b330
Compare
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.
Thanks for root causing the memory increase and optimizing it!
I think we'd want to have a few people review this since it adds a new queue (with its own metrics, for instance) and a new flag.
There's also a consideration around naming those two, especially since there are potentially multiple different archive workers (e.g. GC'ing archived workflows a la archiveTTL
is another one)
e06b330
to
d8d4cd5
Compare
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.
Makes sense to me. @Joibel could you double-check this when you're back from vacation? Specifically as this adds a new queue with metrics. Also if you have any thoughts on the naming
Yes, I'll take a look. |
d8d4cd5
to
48b65e8
Compare
…imited. Fixes #13418 (#13419) Signed-off-by: chenrui <[email protected]>
48b65e8
to
d4ee3fc
Compare
Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: chenrui <[email protected]>
Head branch was pushed to by a user without write access
@Joibel |
@Joibel all checks have passed now, could you merge this PR? thanks |
Thank you for working through this. |
@ChenRussell small reminder on the refactor discussed above |
Yes, I will finish it |
Fix #13418
Motivation
When archiving workflows operations are severely rate-limited,there is a big memory usage increase in workflow controller
Modifications
To solve this problem, we can add a archived_wf_queue to archive the workflow asynchronously when listening the Add and Update events instead of archiving the workflow synchronously
Verification
run locally and tested