Skip to content

Fix project db query for GraphQL pagination

Corinna Gogolok requested to merge 270069_fix_project_db_query_for_graphql_pagination into master

What does this MR do?

Part of #270069

Due to a calculation for ordering the projects on the usage quota page in the storage tab, the GraphQL pagination returned wrong results on the next page click. This change will introduce a new logic in the GraphQL pagination especially for the order for this query.

Note: No changelog as all the storage logic is behind a feature flag.

Query Plan

This will modify the query from !44124 (merged) to make it work correctly with the GraphQL pagination.

The pagination limits the query result to 20, the query plan is without a limit though.

Query:

EXPLAIN SELECT "projects".*,
  (("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) AS excess_storage
FROM "projects"
INNER JOIN routes rs ON rs.source_id = projects.id AND rs.source_type = 'Project'
INNER JOIN "project_statistics" ON "project_statistics"."project_id" = "projects"."id"
WHERE (rs.path LIKE 'gbobject/%')
ORDER BY (("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) DESC,
    "projects"."id" DESC

GraphQL pagination query example:

SELECT "projects".*,
    (("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) AS excess_storage
FROM "projects"
INNER JOIN routes rs ON rs.source_id = projects.id AND rs.source_type = 'Project'
INNER JOIN "project_statistics" ON "project_statistics"."project_id" = "projects"."id"
WHERE (rs.path LIKE 'storage-test/%')
    AND (
        ((("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) < '-10728714240')
        OR (
          (("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) = '-10728714240'
          AND
          "projects"."id" < 120
        )
        OR ((("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) IS NULL)
    )
    AND "projects"."id" != 120
ORDER BY (("project_statistics"."repository_size" + "project_statistics"."lfs_objects_size") - COALESCE("projects"."repository_size_limit", 10737418240, 0)) DESC,
    "projects"."id" DESC
LIMIT 20

Query plan: https://explain.depesz.com/s/Ji4V 

Gather Merge  (cost=105469.93..107600.30 rows=18525 width=770) (actual time=3606.112..3651.777 rows=39840 loops=1)
   Workers Planned: 1
   Workers Launched: 1
   Buffers: shared hit=298371 read=59891 dirtied=1753
   I/O Timings: read=6163.562
   ->  Sort  (cost=104469.92..104516.23 rows=18525 width=770) (actual time=3591.241..3594.332 rows=19920 loops=2)
         Sort Key: (((project_statistics.repository_size + project_statistics.lfs_objects_size) - COALESCE(projects.repository_size_limit, '10737418240'::bigint))) DESC, projects.id DESC
         Sort Method: quicksort  Memory: 11033kB
         Buffers: shared hit=298371 read=59891 dirtied=1753
         I/O Timings: read=6163.562
         ->  Nested Loop  (cost=1.43..103156.76 rows=18525 width=770) (actual time=1.480..3509.735 rows=19920 loops=2)
               Buffers: shared hit=298352 read=59891 dirtied=1753
               I/O Timings: read=6163.562
               ->  Nested Loop  (cost=1.00..93874.17 rows=17979 width=766) (actual time=1.128..2590.813 rows=19920 loops=2)
                     Buffers: shared hit=154176 read=44686 dirtied=1695
                     I/O Timings: read=4662.638
                     ->  Parallel Index Scan using index_routes_on_path_text_pattern_ops on public.routes rs  (cost=0.56..36663.47 rows=18634 width=4) (actual time=0.570..341.971 rows=19920 loops=2)
                           Index Cond: (((rs.path)::text ~>=~ 'gbobject/'::text) AND ((rs.path)::text ~<~ 'gbobject0'::text))
                           Filter: (((rs.path)::text ~~ 'gbobject/%'::text) AND ((rs.source_type)::text = 'Project'::text))
                           Rows Removed by Filter: 0
                           Buffers: shared hit=30277 read=9127 dirtied=44
                           I/O Timings: read=559.028
                     ->  Index Scan using projects_pkey on public.projects  (cost=0.43..3.07 rows=1 width=762) (actual time=0.111..0.111 rows=1 loops=39840)
                           Index Cond: (projects.id = rs.source_id)
                           Buffers: shared hit=123899 read=35559 dirtied=1651
                           I/O Timings: read=4103.610
               ->  Index Scan using index_project_statistics_on_project_id on public.project_statistics  (cost=0.43..0.50 rows=1 width=20) (actual time=0.044..0.044 rows=1 loops=39840)
                     Index Cond: (project_statistics.project_id = projects.id)
                     Buffers: shared hit=144176 read=15205 dirtied=58
                     I/O Timings: read=1500.923

Cold cache:

Time: 3.657 s
  - planning: 1.614 ms
  - execution: 3.656 s
    - I/O read: 6.164 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 298371 (~2.30 GiB) from the buffer pool
  - reads: 59891 (~467.90 MiB) from the OS file cache, including disk I/O
  - dirtied: 1753 (~13.70 MiB)
  - writes: 0

Warm cache:

Time: 342.524 ms
  - planning: 1.549 ms
  - execution: 340.975 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 358260 (~2.70 GiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Toon Claes

Merge request reports