normandy icon indicating copy to clipboard operation
normandy copied to clipboard

Users are not prefetched in RecipeRevisionViewset /api/v3/recipe/

Open peterbe opened this issue 7 years ago • 1 comments

I'm not sure if it matters for v3 but with ?status=enabled isn't that part of the "hot path"? I only have 2 enabled recipes in my local database and they were created, approved and enabled all by the same user (me).

curl http://localhost:8000/api/v3/recipe/?ordering=-last_updated&status=enabled&page=1

I'm guessing it's related to the fetching of the author, approver etc. Theoretically, we should only need this type of query once. And even better if it was once per web request.

select statements for same user 2018-10-23 09:51:52.922 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.924 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.927 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.936 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.939 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.940 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.942 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.948 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.950 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.951 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.953 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.961 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4 2018-10-23 09:51:52.966 EDT [71538] LOG: statement: SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 4

peterbe avatar Oct 23 '18 13:10 peterbe

I'm not sure if it matters for v3 but with ?status=enabled isn't that part of the "hot path"?

This view is not on the hot path, since most of our traffic (Firefox clients) only use the v1 API. It is good to optimize the v3 API, but it isn't a high priority.

You have 13 selects in your log. I expect that they break down like this:

  • one mystery select that I can't explain
  • for each recipe (2)
    • for each of the latest and approved revision, which are probably the same but not cached (2)
      • the author, the approval requester, and the approver (3)

Barring that mystery thirteenth, all of those selects are included in the API (223=12). They could all be unique users as well. It would be great if we could reduce the number of selects required in the case that users are duplicated, and it would be even cooler if we could lump all of them into a single grouped select.

This all makes sense considering we don't prefetch any users on this viewset:

https://github.com/mozilla/normandy/blob/9f9794bfa792962adfddb8f44d92d51e26861894/normandy/recipes/api/v3/views.py#L163-L168

I suspect that adding the user fields described above to the select_related call would do the trick.

mythmon avatar Oct 23 '18 17:10 mythmon