Skip to content

Avoid loading user objects when bulk updating todos

Markus Koller requested to merge 325690-avoid-loading-user-objects-when-bulk-updating-todos into master

What does this MR do?

The method User.count_cache_validity_period was previously behind a feature flag, but now that's gone we can make it constant and avoid loading the user objects.

Follow-up to #325690 (closed)

The FF was removed in !59746 (merged)

Query plans

Relevant queries in...

TodoService#destroy_target:

  • Before:

    • SELECT "users".* FROM "users" WHERE "users"."id" IN (SELECT DISTINCT "todos"."user_id" FROM "todos" WHERE "todos"."target_id" = 423116 AND "todos"."target_type" = 'Issue')

    • https://console.postgres.ai/shared/49487c7e-1a42-4c45-8828-afc6432e38ba

    • Query plan 
      Nested Loop  (cost=5.17..11.67 rows=2 width=1314) (actual time=6.360..6.362 rows=0 loops=1)
  Buffers: shared hit=3 read=4
  I/O Timings: read=6.291
  ->  Unique  (cost=4.73..4.74 rows=2 width=4) (actual time=6.359..6.360 rows=0 loops=1)
        Buffers: shared hit=3 read=4
        I/O Timings: read=6.291
        ->  Sort  (cost=4.73..4.74 rows=2 width=4) (actual time=6.359..6.359 rows=0 loops=1)
              Sort Key: todos.user_id
              Sort Method: quicksort  Memory: 25kB
              Buffers: shared hit=3 read=4
              I/O Timings: read=6.291
              ->  Index Scan using index_todos_on_target_type_and_target_id on public.todos  (cost=0.57..4.72 rows=2 width=4) (actual time=6.338..6.338 rows=0 loops=1)
                    Index Cond: (((todos.target_type)::text = 'Issue'::text) AND (todos.target_id = 423116))
                    Buffers: shared read=4
                    I/O Timings: read=6.291
  ->  Index Scan using users_pkey on public.users  (cost=0.43..3.45 rows=1 width=1314) (actual time=0.000..0.000 rows=0 loops=0)
        Index Cond: (users.id = todos.user_id)

  • After:

    • SELECT DISTINCT "todos"."user_id" FROM "todos" WHERE "todos"."target_id" = 423116 AND "todos"."target_type" = 'Issue'

    • https://console.postgres.ai/shared/ac3d5faf-ada4-4823-b521-fbaa37f18782

    • Query plan 
      Unique  (cost=4.73..4.74 rows=2 width=4) (actual time=0.468..0.470 rows=0 loops=1)
  Buffers: shared hit=3 read=4
  I/O Timings: read=0.393
  ->  Sort  (cost=4.73..4.74 rows=2 width=4) (actual time=0.468..0.469 rows=0 loops=1)
        Sort Key: todos.user_id
        Sort Method: quicksort  Memory: 25kB
        Buffers: shared hit=3 read=4
        I/O Timings: read=0.393
        ->  Index Scan using index_todos_on_target_type_and_target_id on public.todos  (cost=0.57..4.72 rows=2 width=4) (actual time=0.425..0.425 rows=0 loops=1)
              Index Cond: (((todos.target_type)::text = 'Issue'::text) AND (todos.target_id = 423116))
              Buffers: shared read=4
              I/O Timings: read=0.393

TodoService#create_todos:

  • Before:

    • SELECT "todos"."user_id" FROM "todos" WHERE "todos"."user_id" = 423116 AND ("todos"."state" IN ('pending'))

    • https://console.postgres.ai/shared/27a56fc7-3af6-4efd-84a1-4984510defa1

    • Query plan 
      Index Only Scan using index_todos_on_user_id_and_id_pending on public.todos  (cost=0.44..5.05 rows=92 width=4) (actual time=0.455..8.169 rows=175 loops=1)
  Index Cond: (todos.user_id = 423116)
  Heap Fetches: 92
  Buffers: shared hit=28 read=110 dirtied=26
  I/O Timings: read=6.312

  • After:

    • SELECT DISTINCT "todos"."user_id" FROM "todos" WHERE "todos"."user_id" = 423116 AND ("todos"."state" IN ('pending'))

    • https://console.postgres.ai/shared/b150b118-5bfd-4228-acac-1fc8d2b18e50

    • Query plan 
      Unique  (cost=0.44..5.05 rows=92 width=4) (actual time=4.259..96.884 rows=1 loops=1)
  Buffers: shared hit=28 read=110 dirtied=26
  I/O Timings: read=92.800
  ->  Index Only Scan using index_todos_on_user_id_and_id_pending on public.todos  (cost=0.44..5.05 rows=92 width=4) (actual time=4.257..96.847 rows=175 loops=1)
        Index Cond: (todos.user_id = 423116)
        Heap Fetches: 92
        Buffers: shared hit=28 read=110 dirtied=26
        I/O Timings: read=92.800

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

Related to #325690 (closed)

Edited by Markus Koller

Merge request reports