#91 Remove blocking call to sqlite by using aiosqlite module.
Merged by pingou. Opened by cverna.
cverna/mdapi python3_friendly  into  master

Download 91.patch

This commit removes the dependency to sqlalchemy to manage the
sqlite connection and queries. The reason behind is that sqlalchemy
is blocking the event loop in an asyncio context which made each
request needed to have access to the sqlite databases blocking.

The commit replaces sqlalchemy by aiosqlite a async sqlite wrapper
around the sqlite3 module from the standard library.
In order to make it easier to understand the code base, all the
queries used and the classes used to store the queries results
were moved to the mdapi/db.py module.
The views are now stored in the mdapi.views.py and added to the
application router in mdapi/server.py.
The logic which forms the response to the requests based on the
data gathered in the databases is present in mdapi/init.py.
It might make sense to move this to the db.py module in a later
commit.

Before this commit mdapi answered an average of 28.5 requests
per seconds for 100 concurent requests.
After this commit mdapi answers an average of 97 requests
per seconds for 100 concurent requests.

Signed-off-by: Clement Verna cverna@tutanota.com

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

Any reason to remove the spec file?

Why do we remove this one?

We need to use ? instead of {} and provide a tuple as a second argument to query.execute()
Cf: https://docs.python.org/3/library/sqlite3.html 4th exemple

After this commit mdapi answers an average of 97 requests
per seconds for 100 concurent requests.

Out of curiosity, how did you test this?

Any reason to remove the spec file?

Since we deploy this in OpenShift building directly from git, I did not see much value in keeping the spec file.
But happy to keep it if you prefer.

Why do we remove this one?

The application is now run by gunicorn instead of directly aiohttp. This way we can use more than 1 CPU if we wish by starting more than 1 gunicorn worker.

We need to use ? instead of {} and provide a tuple as a second argument to query.execute()
Cf: https://docs.python.org/3/library/sqlite3.html 4th exemple

Oh nice :-) I ll update that.

After this commit mdapi answers an average of 97 requests
per seconds for 100 concurent requests.

Out of curiosity, how did you test this?

Following your blog post http://blog.pingoured.fr/index.php?post/2015/11/19/Introducing-mdapi and using the ab tool on the master branch and on my PR branch.

Any reason to remove the spec file?

Since we deploy this in OpenShift building directly from git, I did not see much value in keeping the spec file.
But happy to keep it if you prefer.

It's where we have the changelog at the moment, so I'd prefer that we keep it, at least until we find another place for the changelog.

The application is now run by gunicorn instead of directly aiohttp. This way we can use more than 1 CPU if we wish by starting more than 1 gunicorn worker.

Let's put a note in the README on how to get mdapi running then (or we just update this script to run gunicorn). I was using it to run it locally :-p

using the ab tool on the master branch and on my PR branch.

Cool, thanks :)

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

The application is now run by gunicorn instead of directly aiohttp. This way we can use more than 1 CPU if we wish by starting more than 1 gunicorn worker.

Let's put a note in the README on how to get mdapi running then (or we just update this script to run gunicorn). I was using it to run it locally :-p

I did update the README --> https://pagure.io/mdapi/pull-request/91#_4__6, but yes I can also just modify the mdapi-run script to start gunicorn. I don't have a strong preference :smile:

If we keep the spec file, let's keep it in the tarballs as well :)

Cf below :)

If we keep the spec file, we need to keep this one as well (and potentially adjust it)

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

I have added back the .spec, .service, and mdapi-run and changed mdapi-run to start gunicorn instead of aiohttp directly.

I still think it would be nice to remove these file and even the setup.py which isn't really needed. But that can be done in another PR which will make it easier to review :smile:

Any reason not to use latest here?

This is what I got when trying to run the container in podman (btw, we should document how to do this in the README, especially the make available the sqlite databases to the container)

container_linux.go:346: starting container process caused "exec: \"gunicorn\": executable file not found in $PATH": OCI runtime command not found error

Any reason not to use latest here?

Using latest is generally a bad idea since you don't have control which version of the base image your run. Without knowing it you will jump from f31 to f32 and so on.
It is much better to be in control of when you want to upgrade you base image.

This is what I got when trying to run the container in podman (btw, we should document how to do this in the README, especially the make available the sqlite databases to the container)

Yes, I have not use the container for development just a simple venv. The Dockerfile here is what is used by OpenShift to build the container.

container_linux.go:346: starting container process caused "exec: \"gunicorn\": executable file not found in $PATH": OCI runtime command not found error

Ha yes gunicorn is missing from the image, fixing that

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

I've needed these changes to the Dockerfile to get it to run:

-ENTRYPOINT ["gunicorn", "mdapi.server:init_app", "--bind 0.0.0.0:8080", "--worker-class", "aiohttp.GunicornUVLoopWebWorker", "-w", "2"]
+WORKDIR /code
+ENTRYPOINT ["gunicorn", "--bind", "0.0.0.0:8080", "mdapi.server:init_app", "--worker-class", "aiohttp.GunicornUVLoopWebWorker", "-w", "2"]

Then I ran into:

  File "/code/mdapi/views.py", line 45, in get_pkg
    pkg, repotype = await _get_pkg(branch, name)
  File "/code/mdapi/__init__.py", line 96, in _get_pkg
    async with db.execute(GET_PACKAGE, name) as cursor:
  File "/usr/local/lib/python3.7/site-packages/aiosqlite/context.py", line 35, in __aenter__
    self._obj = await self._coro
  File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 209, in execute
    cursor = await self._execute(self._conn.execute, sql, parameters)
  File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 167, in _execute
    return await future
  File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 153, in run
    result = function()
sqlite3.ProgrammingError: Incorrect number of bindings supplied. The current statement uses 0, and there are 6 supplied.

trying to access http://127.0.0.1:8080/rawhide/pkg/kernel

response is undefined here :)

2 new commits added

  • Remove blocking call to sqlite by using aiosqlite module.
  • Replace old string formating by f-strings

I've needed these changes to the Dockerfile to get it to run:
-ENTRYPOINT ["gunicorn", "mdapi.server:init_app", "--bind 0.0.0.0:8080", "--worker-class", "aiohttp.GunicornUVLoopWebWorker", "-w", "2"]
+WORKDIR /code
+ENTRYPOINT ["gunicorn", "--bind", "0.0.0.0:8080", "mdapi.server:init_app", "--worker-class", "aiohttp.GunicornUVLoopWebWorker", "-w", "2"]

Then I ran into:
File "/code/mdapi/views.py", line 45, in get_pkg
pkg, repotype = await _get_pkg(branch, name)
File "/code/mdapi/init.py", line 96, in _get_pkg
async with db.execute(GET_PACKAGE, name) as cursor:
File "/usr/local/lib/python3.7/site-packages/aiosqlite/context.py", line 35, in aenter
self._obj = await self._coro
File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 209, in execute
cursor = await self._execute(self._conn.execute, sql, parameters)
File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 167, in _execute
return await future
File "/usr/local/lib/python3.7/site-packages/aiosqlite/core.py", line 153, in run
result = function()
sqlite3.ProgrammingError: Incorrect number of bindings supplied. The current statement uses 0, and there are 6 supplied.

trying to access http://127.0.0.1:8080/rawhide/pkg/kernel

Ok, I did not use correctly the sqlite formating with '?', Should be fixed now :smile:

response is undefined here :)

Updated :)

I still think my changes to the Dockerfile are needed, especially the WORKDIR one as otherwise gunicorn doesn't find the mdapi module

I still think my changes to the Dockerfile are needed, especially the WORKDIR one as otherwise gunicorn doesn't find the mdapi module

Nevermind I see it's there, somehow my pull did not have it when I did it :(

Thanks

I have deployed this branch in communishift so we could get a idea of the performances (no nfs storage).

ab -c 100 -n 1000 http://mdapi-git-mdapi.apps.os.fedorainfracloud.org/rawhide/pkg/kernel
This is ApacheBench, Version 2.3 <$Revision: 1843412 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/
Benchmarking mdapi-git-mdapi.apps.os.fedorainfracloud.org (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Completed 600 requests
Completed 700 requests
Completed 800 requests
Completed 900 requests
Completed 1000 requests
Finished 1000 requests
Server Software:        Python/3.7
Server Hostname:        mdapi-git-mdapi.apps.os.fedorainfracloud.org
Server Port:            80
Document Path:          /rawhide/pkg/kernel
Document Length:        1091 bytes
Concurrency Level:      100
Time taken for tests:   5.308 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      1371000 bytes
HTML transferred:       1091000 bytes
Requests per second:    188.41 [#/sec] (mean)
Time per request:       530.768 [ms] (mean)
Time per request:       5.308 [ms] (mean, across all concurrent requests)
Transfer rate:          252.25 [Kbytes/sec] received
Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:      161  167   7.9    166     245
Processing:   194  307 131.1    254     775
Waiting:      193  306 131.1    254     774
Total:        357  474 132.7    420     943
Percentage of the requests served within a certain time (ms)
  50%    420
  66%    464
  75%    524
  80%    545
  90%    623
  95%    871
  98%    917
  99%    934
 100%    943 (longest request)

I could try to build the master branch in communishift to compare. Also I expect these number to be higher in stg and prod because of the NFS storage access.

If you have the time, could you run the master branch on communishift as well?
It'll give us a baseline to compare things to before merging this.

(actually, we could just run this against the current staging instance of mdapi for comparison)

Here is the outcome against the current mdapi instance in staging:

 ab -c 100 -n 1000 https://mdapi.stg.fedoraproject.org/rawhide/pkg/kernel 
This is ApacheBench, Version 2.3 <$Revision: 1843412 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/
Benchmarking mdapi.stg.fedoraproject.org (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Completed 600 requests
Completed 700 requests
Completed 800 requests
Completed 900 requests
Completed 1000 requests
Finished 1000 requests
Server Software:        Python/3.7
Server Hostname:        mdapi.stg.fedoraproject.org
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-AES128-GCM-SHA256,4096,128
Server Temp Key:        X25519 253 bits
TLS Server Name:        mdapi.stg.fedoraproject.org
Document Path:          /rawhide/pkg/kernel
Document Length:        1091 bytes
Concurrency Level:      100
Time taken for tests:   200.136 seconds
Complete requests:      1000
Failed requests:        963
   (Connect: 0, Receive: 0, Length: 963, Exceptions: 0)
Non-2xx responses:      963
Total transferred:      1924658 bytes
HTML transferred:       1422539 bytes
Requests per second:    5.00 [#/sec] (mean)
Time per request:       20013.640 [ms] (mean)
Time per request:       200.136 [ms] (mean, across all concurrent requests)
Transfer rate:          9.39 [Kbytes/sec] received
Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:      490  726 271.7    626    2047
Processing:   165 17841 14315.9  30165   38959
Waiting:      164 17827 14305.5  30165   38959
Total:        659 18567 14410.0  30662   39490
Percentage of the requests served within a certain time (ms)
  50%  30662
  66%  30845
  75%  31070
  80%  31130
  90%  31455
  95%  31545
  98%  32346
  99%  32398
 100%  39490 (longest request)

Alright, let's merge this and push it to staging and see how it behaves :)

Pull-Request has been merged by pingou

And here are the numbers for the new code in staging:

 ab -c 100 -n 1000 https://mdapi.stg.fedoraproject.org/rawhide/pkg/kernel
This is ApacheBench, Version 2.3 <$Revision: 1843412 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/
Benchmarking mdapi.stg.fedoraproject.org (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Completed 600 requests
Completed 700 requests
Completed 800 requests
Completed 900 requests
Completed 1000 requests
Finished 1000 requests
Server Software:        Python/3.7
Server Hostname:        mdapi.stg.fedoraproject.org
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-AES128-GCM-SHA256,4096,128
Server Temp Key:        X25519 253 bits
TLS Server Name:        mdapi.stg.fedoraproject.org
Document Path:          /rawhide/pkg/kernel
Document Length:        1091 bytes
Concurrency Level:      100
Time taken for tests:   33.458 seconds
Complete requests:      1000
Failed requests:        1
   (Connect: 0, Receive: 0, Length: 1, Exceptions: 0)
Non-2xx responses:      1
Total transferred:      1721844 bytes
HTML transferred:       1090328 bytes
Requests per second:    29.89 [#/sec] (mean)
Time per request:       3345.809 [ms] (mean)
Time per request:       33.458 [ms] (mean, across all concurrent requests)
Transfer rate:          50.26 [Kbytes/sec] received
Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:      497 1291 423.1   1142    3534
Processing:   166 1649 420.7   1800    2495
Waiting:      166 1649 420.7   1800    2495
Total:        944 2940 376.5   2938    5300
Percentage of the requests served within a certain time (ms)
  50%   2938
  66%   2995
  75%   3051
  80%   3088
  90%   3137
  95%   3168
  98%   3196
  99%   5222
 100%   5300 (longest request)

So we went from

Requests per second:    5.00 [#/sec] (mean)
Time per request:       20013.640 [ms] (mean)

to

Requests per second:    29.89 [#/sec] (mean)
Time per request:       3345.809 [ms] (mean)

The production numbers:

Before the upgrade

$ ab -c 100 -n 1000 https://mdapi.fedoraproject.org/rawhide/pkg/kernel                        
This is ApacheBench, Version 2.3 <$Revision: 1843412 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/
Benchmarking mdapi.fedoraproject.org (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Completed 600 requests
Completed 700 requests
Completed 800 requests
Completed 900 requests
Completed 1000 requests
Finished 1000 requests
Server Software:        Python/3.7
Server Hostname:        mdapi.fedoraproject.org
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-AES128-GCM-SHA256,4096,128
Server Temp Key:        X25519 253 bits
TLS Server Name:        mdapi.fedoraproject.org
Document Path:          /rawhide/pkg/kernel
Document Length:        1036 bytes
Concurrency Level:      100
Time taken for tests:   182.775 seconds
Complete requests:      1000
Failed requests:        943
   (Connect: 0, Receive: 0, Length: 943, Exceptions: 0)
Non-2xx responses:      943
Total transferred:      1956607 bytes
HTML transferred:       1460389 bytes
Requests per second:    5.47 [#/sec] (mean)
Time per request:       18277.459 [ms] (mean)
Time per request:       182.775 [ms] (mean, across all concurrent requests)
Transfer rate:          10.45 [Kbytes/sec] received
Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:      355  560 177.5    510    1165
Processing:   362 16992 13980.8  25634   31538
Waiting:      361 16991 13981.4  25634   31538
Total:        724 17553 13878.6  26163   31900
Percentage of the requests served within a certain time (ms)
  50%  26163
  66%  30734
  75%  30776
  80%  30876
  90%  30919
  95%  31314
  98%  31378
  99%  31390
 100%  31900 (longest request)

After the upgrade:

$ ab -c 100 -n 1000 https://mdapi.fedoraproject.org/rawhide/pkg/kernel
This is ApacheBench, Version 2.3 <$Revision: 1843412 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/
Benchmarking mdapi.fedoraproject.org (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Completed 600 requests
Completed 700 requests
Completed 800 requests
Completed 900 requests
Completed 1000 requests
Finished 1000 requests
Server Software:        Python/3.7
Server Hostname:        mdapi.fedoraproject.org
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-AES128-GCM-SHA256,4096,128
Server Temp Key:        X25519 253 bits
TLS Server Name:        mdapi.fedoraproject.org
Document Path:          /rawhide/pkg/kernel
Document Length:        1036 bytes
Concurrency Level:      100
Time taken for tests:   30.205 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      1658914 bytes
HTML transferred:       1036000 bytes
Requests per second:    33.11 [#/sec] (mean)
Time per request:       3020.499 [ms] (mean)
Time per request:       30.205 [ms] (mean, across all concurrent requests)
Transfer rate:          53.63 [Kbytes/sec] received
Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:      377  765 236.0    707    1593
Processing:   505 2013 859.7   1888    5511
Waiting:      505 2012 859.7   1888    5511
Total:       1138 2777 893.3   2603    6283
Percentage of the requests served within a certain time (ms)
  50%   2603
  66%   2714
  75%   2841
  80%   2959
  90%   3383
  95%   6115
  98%   6233
  99%   6254
 100%   6283 (longest request)

So we went from:

Requests per second:    5.47 [#/sec] (mean)
Time per request:       18277.459 [ms] (mean)

to:

Requests per second:    33.11 [#/sec] (mean)
Time per request:       3020.499 [ms] (mean)

In production as well :)

Metadata