Bug 54575 - Kelvin REST API should use more than 1 CPU core
Summary: Kelvin REST API should use more than 1 CPU core
Status: VERIFIED FIXED
Alias: None
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
Version: UCS@school 5.0
Hardware: Other Linux
: P5 normal
Target Milestone: ---
Assignee: Tobias Wenzel
QA Contact: Ole Schwiegert
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-22 16:56 CET by Tobias Wenzel
Modified: 2022-03-29 11:35 CEST (History)
1 user (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Customer ID:
Max CVSS v3 score:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Wenzel univentionstaff 2022-03-22 16:56:25 CET 
As a UCS@school systems administrator I want to be able to configure the amount of processes/CPU cores the Kelvin app utilizes.


When the UDM REST API scales to more than 1 CPU core (https://forge.univention.org/bugzilla/show_bug.cgi?id=50050) the Kelvin REST API can become the bottleneck as it currently runs on one CPU only.

Instead of spawning the uvicorn process directly, a Gunicorn instance can spawn multiple processes. See FastAPI documentation: https://fastapi.tiangolo.com/deployment/server-workers/#gunicorn-with-uvicorn-workers


Basically it is just instead of executing uvicron .., to execute gunicorn --workers 16 --worker-class uvicorn.workers.UvicornWorker ....
Comment 2 Tobias Wenzel univentionstaff 2022-03-24 08:56:33 CET 
I created a MR here: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/82

- [x] added new app setting
- [x] kelvin api can be started with multiple workers without errors
- [x] Changelog + Documentation adapted
- [x] The Dockercontainer build locally
- [ ] published in test appcenter
Comment 3 Tobias Wenzel univentionstaff 2022-03-25 14:18:13 CET 
fix -> see merge request

I added some documentation in the operations manual here https://git.knut.univention.de/univention/id-broker/operations-manual/-/merge_requests/6

jenkins jobs were modified here

[5.0-1] ea10df2699 Bug #54575: use multiple processes for kelvin api in id broker job
Comment 4 Tobias Wenzel univentionstaff 2022-03-25 15:01:11 CET 
as discussed, I merged to the main branches

[feature/kelvin] 36f93fb0f Bug #54575: add app setting to set workers for kelvin

[main] da5e839 Merge branch 'twenzel/54575_multiple_cores' into 'main'
[main] a53ca13 Bug #54575: document number of processes
Comment 5 Daniel Tröder univentionstaff 2022-03-28 08:29:50 CEST 
Please remove all mentioning of "gunicorn" from user facing documentation.
It is an unnecessary and for a customer confusing technical detail.

If you wish to support developers or supporters, then add a dedicated "architecture" section to the manual.

Not important, but if you're at it, please prefix "cores" with "CPU ".

The commit message "add app setting to set workers for kelvin" is a bit misleading. Git messages are for developers. While it is true that app setting were added, the really important change is an architectural one.

The file appcenter/README_UPDATE_EN received an additional changelog entry in the previous app version.
Comment 6 Tobias Wenzel univentionstaff 2022-03-28 11:16:36 CEST 
Thanks for the remarks!

[feature/kelvin] c56cc0ff2 Bug #54575: remove gunicorn mention of docu
Comment 7 Ole Schwiegert univentionstaff 2022-03-29 11:35:26 CEST 
Kelvin App in TestAppCenter 1.5.4

Changes work, doc adapated, app setting added

-- VERIFIED