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

quota: improve performance of quota updater #200

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

mdonadoni
Copy link
Member

quota: improve performance of quota updater

Improve the performance of the quota updater by "expunging" all
workflows before starting to update quotas, as the Workflow table does
not need to be modified, thus making commits much faster. Also refactor
the various update functions to make their behaviour consistent with
each other.

Partially addresses #193

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #200 (8cc26f7) into master (c758c83) will increase coverage by 0.76%.
The diff coverage is 71.11%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #200      +/-   ##
==========================================
+ Coverage   73.55%   74.32%   +0.76%     
==========================================
  Files           7        7              
  Lines         832      892      +60     
==========================================
+ Hits          612      663      +51     
- Misses        220      229       +9     
Files Changed Coverage Δ
reana_db/cli.py 0.00% <0.00%> (ø)
reana_db/utils.py 76.10% <77.77%> (+4.34%) ⬆️
reana_db/models.py 90.43% <100.00%> (+0.01%) ⬆️

@mdonadoni
Copy link
Member Author

Small note for this PR: this PR maintains the current approach of having one commit per workflow/user, but this will not scale with an even bigger number of workflows and/or users.
Ideas for improvements:

  • use bulk operations
  • consider only workflows that were actually "modified" and not all of them
  • run the quota updates asynchronously

@tiborsimko
Copy link
Member

Some thoughts on the possible future options:

use bulk operations

If we make only one commit at the end of the session, and we would have to update many rows, and this last commit would fail for a reason-or-another, how would the rollback situation look? Might not be so interesting as the other two approaches?

consider only workflows that were actually "modified" and not all of them

This would be definitely good to do longer term, i.e. process only "modified" workspaces, and having an option to process "everything" once in a blue moon to catch up any possible troubles.

However for operations such as rm we wouldn't know just by checking the workspace contents whether some file was removed from notebook session terminal. So we'd have to take into account not only the workflow file manipulations, but also when a terminal might have been opened/closed on some workspace.

run the quota updates asynchronously

Yes, it might be good to have another queue where we register "please process quota for this workflow" kind of requests and have another daemon consumer process running permanently in the background that would consume these messages and doing the necessary quota updates as they arrive. In this way we would also address the previous item about doing only those updates that are actually needed. (Even though there may still be corner cases such a a user leaving a notebook terminal opened for many days without closing.) This requires quite some development though, but we might want to ticketise the ideas for the future!

# This makes `Session.commit()` much faster
for workflow in workflows:
Session.expunge(workflow)
timer = Timer("Workflow disk", total=len(workflows))
Copy link
Member

Choose a reason for hiding this comment

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

Cosmetics: the log messages currently look like:

2023-08-25 14:22:22,682 | root | MainThread | INFO | Workflow disk progress: 8/8  elapsed: 0.066s  est.total: 0.066s  per event: 0.008s
2023-08-25 14:22:22,703 | root | MainThread | INFO | User disk progress: 3/3  elapsed: 0.019s  est.total: 0.019s  per event: 0.006s
Users disk quota usage updated successfully.
2023-08-25 14:22:22,721 | root | MainThread | INFO | Workflow CPU progress: 8/8  elapsed: 0.015s  est.total: 0.015s  per event: 0.002s
2023-08-25 14:22:22,738 | root | MainThread | INFO | User CPU progress: 3/3  elapsed: 0.016s  est.total: 0.016s  per event: 0.005s
Users cpu quota usage updated successfully.

We may want to improve the INFO message (it's not "CPU progress"!) by using something like:

  • "Workflow disk quota usage update progress: 8/8"

and clarify the final message:

  • "Disk quota usage updated successfully for all users and workflows."

Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

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

Works well locally 👍

Improve the performance of the quota updater by "expunging" all
workflows before starting to update quotas, as the Workflow table does
not need to be modified, thus making commits much faster. Also refactor
the various update functions to make their behaviour consistent with
each other.

Partially addresses reanahub#193
@mdonadoni mdonadoni merged commit 8cc26f7 into reanahub:master Sep 1, 2023
12 checks passed
@mdonadoni mdonadoni deleted the quota-updater-perf branch September 1, 2023 08:10
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