PR#942: frontend: optimize /backend/pending-jobs/ json rendering - copr/copr

copr / copr

#942 frontend: optimize /backend/pending-jobs/ json rendering

Merged 4 years ago by msuchy. Opened 4 years ago by praiskup.

Unknown source optimized-pending-jobs into master

frontend: optimize /backend/pending-jobs/ json rendering

Pavel Raiskup • 4 years ago

548a9a0

frontend/coprs_frontend/coprs/logic/builds_logic.py

file modified

+7 -1

		`@@ -7,6 +7,7 @@`
		`import requests`

		`from sqlalchemy.sql import text`
		`+ from sqlalchemy.orm import joinedload`
		`from sqlalchemy import or_`
		`from sqlalchemy import and_`
		`from sqlalchemy import func, desc`
		`@@ -282,7 +283,12 @@`

		`@classmethod`
		`def get_pending_build_tasks(cls, background=None):`
		`- query = (models.BuildChroot.query.join(models.Build)`
		`+ query = (models.BuildChroot.query`
		`+ .outerjoin(models.Build)`
		`+ .outerjoin(models.CoprDir)`
		`+ .outerjoin(models.Package, models.Package.id == models.Build.package_id)`
		`+ .options(joinedload('build').joinedload('copr_dir'),`
		`+ joinedload('build').joinedload('package'))`
		`.filter(models.Build.canceled == false())`
		`.filter(or_(`
		`models.BuildChroot.status == StatusEnum("pending"),`

frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py

file modified

-6

		`@@ -91,7 +91,6 @@`
		`if not task:`
		`return None`

		`- build_config = helpers.generate_build_config(task.build.copr, task.mock_chroot.name)`
		`build_record = None`
		`try:`
		`build_record = {`
		`@@ -115,11 +114,6 @@`
		`"fetch_sources_only": True,`
		`"package_name": task.build.package.name,`
		`"package_version": task.build.pkg_version,`
		`- "repos": build_config.get("repos"),`
		`- "buildroot_pkgs": build_config.get("additional_packages"),`
		`- "use_bootstrap_container": build_config.get("use_bootstrap_container"),`
		`- "with_opts": build_config.get("with_opts"),`
		`- "without_opts": build_config.get("without_opts"),`
		`}`

		`except Exception as err:`

praiskup commented 4 years ago

Make sure that get_build_record() doesn't generate ORM sub-queries, so
the whole bunch of builds is loaded by single (joinedload'ed) query.

As a part of ^^, also drop the build_config part from the result which
generated whole bunch of queries providing data that wasn't used on
backend.

praiskup commented 4 years ago

Some measurement on my box:

After:

$ time curl localhost:55555/backend/pending-jobs/ | jq | grep owner | grep owner | sort | uniq -c
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 4337k  100 4337k    0     0  1819k      0  0:00:02  0:00:02 --:--:-- 1818k
      2     "project_owner": "che",
   5902     "project_owner": "iucar",
      1     "project_owner": "jesonlay",
      1     "project_owner": "praiskup",
      5     "project_owner": "xxmitsu",

real    0m2.518s
user    0m0.256s
sys     0m0.104s

Before:

$ time curl localhost:55555/backend/pending-jobs/ | jq | grep owner | grep owner | sort | uniq -c
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 6056k  100 6056k    0     0   350k      0  0:00:17  0:00:17 --:--:-- 1484k
      2     "project_owner": "che",
   5902     "project_owner": "iucar",
      1     "project_owner": "jesonlay",
      1     "project_owner": "praiskup",
      5     "project_owner": "xxmitsu",

real    0m17.469s
user    0m0.393s
sys     0m0.090s

Most of the time was spent on O(N) sub-queries (N==number of builds). The rest
of the 2.5 seconds is mostly O(N) work on generating json from single database
query, which itself takes ~50ms. This would be harder, mostly micro
optimization because only backend is supposed to consume that route.

rebased onto 4c82b6a735c8c654a5fa08323aaab92936f40eea

4 years ago

rebased onto 548a9a0

4 years ago