fix!: lipome compute task optim by SdgJlbl · Pull Request #613 · Substra/substra-backend

SdgJlbl · 2023-03-09T10:51:25Z

Companion PR

Breaking changes

The endpoint /compute_plans/<pkey>/tasks now returns the ComputeTask without their associated inputs and outputs.
The endpoint /tasks now returns the ComputeTask without their associated inputs and outputs.
The endpoint /tasks/<pkey> still returns the full view of the ComputeTask (including inputs and outputs).

Description

Fixes slow page display (list view of compute tasks linked to a compute plan).

Two issues have been fixed:

add some missing prefetch_related statements
duplicate the ComputeTaskSerializer to have one for the list view and one for the detail view.

The serializer for the list view does not contain any information about inputs and outputs, since these information are not used currently and make a lot of queries to get nested objects.
The serializer for the detail view is the same as it was before (optimised queries though).

How has this been tested?

This has been tested manually for the frontend part, comparing the version actually in production with a local cluster built on my branch. Pages that were checked:

/compute_plans
/compute_plans//task
/compute_plans//task/
/tasks
/task/

For the backend, I have added non-regression tests that check that the number of queries is in O(1). The so-called n+1 queries issue is caused by some missing prefetch_related statement that causes the number of queries to be linear in the number of items. In our case, because of nested items, we actually have a complexity that is higher than linear without the prefetch_related.

Impact

URL	nb items	main	this branch
/compute_plans//tasks	10 / 60 tasks	118/672	14
/tasks	10/60 tasks	38 / 142	14
/task/	4 / 10 inputs	122 / 182	18

TODO

Evaluate impact to delegate the querying of inputs / outputs to the SDK
Update changelog

Checklist

changelog was updated with notable changes
documentation was updated

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

SdgJlbl · 2023-03-14T10:38:44Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon

Owlfred · 2023-03-14T10:38:46Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

“Rien ne sert de courir ; il faut partir à point.” ― Jean de La Fontaine (Le Lièvre et la Tortue)

guilhem-barthes · 2023-03-14T10:28:20Z

+        print("response\n", response.json()[0])
+        print("expected\n", expected_response[0])
+        print("----\n")
+        print(response.json()[0]["key"])
+        print("----\n")


Is it meant to stay in the test suite?

SdgJlbl · 2023-03-14T13:54:53Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon

Owlfred · 2023-03-14T13:54:55Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

I'm sorry.

SdgJlbl · 2023-03-14T17:22:01Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon

Owlfred · 2023-03-14T17:22:04Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

That's a shame.

SdgJlbl · 2023-03-21T07:43:15Z

/e2e --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue

Owlfred · 2023-03-21T07:43:20Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

Oh well.

SdgJlbl · 2023-03-21T17:18:12Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue, substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-21T17:18:15Z

Error: Invalid ref

Usage: /e2e [options] [help]

/e2e may appear anywhere as long as it is on its own line

Options:
  --refs <value>                                         Extra refs (branch or tag) with format REPO=GIT_REF,REPO=GIT_REF.
  Supported repositories: hlf-k8s, orchestrator, substra-backend, substra-frontend, substra-tools, substrafl, substra, substra-tests, substra-ci.
  Example: /e2e --refs substra-backend=some_branch,orchestrator=some_tag (default: {})
  --tests-to-run, --tests <tests-to-run>                 Comma-separated list of tests to run. Valid options: sdk,substrafl,frontend or NONE. (default: "sdk")
  --benchmarks-to-run, --benchmarks <benchmarks-to-run>  Comma-separated list of workflows tests to run. Valid options: mnist, camelyon or NONE. (default: "NONE")
  --orchestrator-mode, --mode <orchestrator-mode>        Comma-separated list of orchestrator modes to run tests for. Valid options: standalone,distributed (default: "standalone,distributed")
  -h, --help                                             display help for command

SdgJlbl · 2023-03-21T17:19:06Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-21T17:19:09Z

End to end tests: ❌ FAILURE

It'll stay between us, no one needs to know.

SdgJlbl · 2023-03-21T17:27:22Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-21T17:27:25Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

“You shall not pass!” ― Gandalf, The Lord of the Rings, The Fellowship of the Ring

SdgJlbl · 2023-03-22T09:11:47Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-22T09:11:50Z

End to end tests: ✔️ SUCCESS

“Shaken, not stirred.” ― James Bond, Goldfinger

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

guilhem-barthes · 2023-03-23T13:17:08Z

-                models.When(start_date__isnull=True, then=0),
-                default=Extract(Coalesce("end_date", Now()) - models.F("start_date"), "epoch"),
+        return (
+            ComputeTask.objects.filter(channel=get_channel_name(self.request))


Is there a difference between using ComputeTask.objects and super().get_queryset()?

I don't think so, I feel like it's slightly more explicit not to use super() given that there is some multiple inheritance with mixins here, but maybe I'm missing some under-the-hood optimization...

guilhem-barthes

LGTM!

SdgJlbl · 2023-03-23T16:30:00Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon

Owlfred · 2023-03-23T16:30:05Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

“Just Keep Swimming!” ― Dory, Finding Nemo

SdgJlbl · 2023-03-24T07:44:55Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-24T07:44:58Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

“Just Keep Swimming!” ― Dory, Finding Nemo

SdgJlbl · 2023-03-24T09:55:02Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-24T09:55:05Z

End to end tests: ❌ FAILURE

“Boy, that escalated quickly.” ― Ron Burgundy, Anchorman: The Legend of Ron Burgundy

SdgJlbl · 2023-03-24T10:01:49Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-24T10:01:52Z

End to end tests: ❌ FAILURE

Jobs status:

Tests Benchmark: ❌
Tests Distributed: ❌
Tests Standalone: ❌

“You shall not pass!” ― Gandalf, The Lord of the Rings, The Fellowship of the Ring

SdgJlbl · 2023-03-24T11:03:56Z

/e2e --tests sdk,frontend,substrafl --benchmarks mnist,camelyon --refs substra=fix/compute-task-performance-issue,substra-tests=fix/compute-task-performance-issue

Owlfred · 2023-03-24T11:03:58Z

End to end tests: ✔️ SUCCESS

“It’s alive! It’s alive!” ― Henry Frankenstein, Frankenstein

github-actions Bot added the api label Mar 9, 2023

SdgJlbl added 3 commits March 10, 2023 20:16

fix: type annotation

b3509db

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: add missing prefetch_related

ff800af

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: add n+1 queries non-regression test

211c67c

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

SdgJlbl force-pushed the fix/lipome-compute-task-optim branch from feb878f to 84e0b0b Compare March 10, 2023 19:19

github-actions Bot added compute-engine documentation Improvements or additions to documentation labels Mar 10, 2023

SdgJlbl changed the base branch from fix/lipome-optimization to main March 10, 2023 19:20

SdgJlbl changed the title ~~Fix/lipome compute task optim~~ fix: lipome compute task optim Mar 10, 2023

github-actions Bot removed documentation Improvements or additions to documentation compute-engine labels Mar 10, 2023

SdgJlbl added 4 commits March 14, 2023 10:54

fix: duplicate compute task serializer

7cc7413

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: post-rebase fixes

2ee7da3

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: fix tests

23f097a

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: linter

de5250f

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

SdgJlbl force-pushed the fix/lipome-compute-task-optim branch from 2be79ad to de5250f Compare March 14, 2023 10:02

SdgJlbl marked this pull request as ready for review March 14, 2023 10:03

SdgJlbl requested a review from a team as a code owner March 14, 2023 10:03

SdgJlbl requested review from Milouu, ThibaultFy, guilhem-barthes and oleobal March 14, 2023 10:03

guilhem-barthes reviewed Mar 14, 2023

View reviewed changes

SdgJlbl force-pushed the fix/lipome-compute-task-optim branch from 077a79f to 13ef406 Compare March 14, 2023 17:21

This was referenced Mar 22, 2023

fix: add summary task pydantic validator Substra/substra#350

Merged

fix: amend end-to-end tests to work with the new API (cf substra-back… Substra/substra-tests#248

Merged

SdgJlbl added 2 commits March 22, 2023 14:38

fix: fix end-to-end tests

45aa625

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

fix: add changelog

5bea4da

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

SdgJlbl force-pushed the fix/lipome-compute-task-optim branch from 7dddb68 to 5bea4da Compare March 22, 2023 13:55

SdgJlbl marked this pull request as ready for review March 22, 2023 13:57

guilhem-barthes reviewed Mar 23, 2023

View reviewed changes

guilhem-barthes approved these changes Mar 23, 2023

View reviewed changes

guilhem-barthes merged commit d7beb02 into main Mar 24, 2023

guilhem-barthes deleted the fix/lipome-compute-task-optim branch March 24, 2023 14:09

Conversation

SdgJlbl commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Companion PR

Breaking changes

Description

How has this been tested?

Impact

TODO

Checklist

Uh oh!

SdgJlbl commented Mar 14, 2023

Uh oh!

Owlfred commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guilhem-barthes Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

SdgJlbl Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

SdgJlbl commented Mar 14, 2023

Uh oh!

Owlfred commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 14, 2023

Uh oh!

Owlfred commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 21, 2023

Uh oh!

Owlfred commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 21, 2023

Uh oh!

Owlfred commented Mar 21, 2023

Uh oh!

SdgJlbl commented Mar 21, 2023

Uh oh!

Owlfred commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 21, 2023

Uh oh!

Owlfred commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 22, 2023

Uh oh!

Owlfred commented Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guilhem-barthes Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

SdgJlbl Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

guilhem-barthes left a comment

Choose a reason for hiding this comment

Uh oh!

SdgJlbl commented Mar 23, 2023

Uh oh!

Owlfred commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 24, 2023

Uh oh!

Owlfred commented Mar 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 24, 2023

Uh oh!

Owlfred commented Mar 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SdgJlbl commented Mar 9, 2023 •

edited

Loading

Owlfred commented Mar 14, 2023 •

edited

Loading

Owlfred commented Mar 14, 2023 •

edited

Loading

Owlfred commented Mar 14, 2023 •

edited

Loading

Owlfred commented Mar 21, 2023 •

edited

Loading

Owlfred commented Mar 21, 2023 •

edited

Loading

Owlfred commented Mar 21, 2023 •

edited

Loading

Owlfred commented Mar 22, 2023 •

edited

Loading

Owlfred commented Mar 23, 2023 •

edited

Loading

Owlfred commented Mar 24, 2023 •

edited

Loading

Owlfred commented Mar 24, 2023 •

edited

Loading

Owlfred commented Mar 24, 2023 •

edited

Loading

Owlfred commented Mar 24, 2023 •

edited

Loading