Skip to content

Speed up SSH key expiration notification queries

Max Woolf requested to merge 331101-expiry-notification-worker-slowness into master

What does this MR do?

  • Changes the batch size when finding users with expired SSH keys which seems to alter the query plan created by postgres and executes much faster.
  • This is not the most elegant solution although it does work fairly consistently. I'm open to better ideas to achieve the same result.

Query Plans

Before After
https://explain.depesz.com/s/Kdmq https://explain.depesz.com/s/f7sXy

Query

SELECT
        "users" . *
    FROM
        "users"
    WHERE
        (
            EXISTS (
                SELECT
                        1
                    FROM
                        "keys"
                    WHERE
                        (
                            keys.user_id = users.id
                        )
                        AND (
                            DATE (
                                expires_at AT TIME ZONE 'UTC'
                            ) = CURRENT_DATE
                            AND expiry_notification_delivered_at IS NULL
                        )
            )
        )
    ORDER BY
        "users" . "id" ASC LIMIT 10000

Database Lab

Before

Time: 15.078 s  
  - planning: 0.788 ms  
  - execution: 15.077 s (estimated* for prod: 9.379...14.559 s)  
    - I/O read: 18.228 s  
    - I/O write: N/A  
  
Shared buffers:  
  - hits: 7737502 (~59.00 GiB) from the buffer pool  
  - reads: 566323 (~4.30 GiB) from the OS file cache, including disk I/O  
  - dirtied: 53345 (~416.80 MiB)  
  - writes: 0  

After

Time: 23.413 ms  
  - planning: 6.808 ms  
  - execution: 16.605 ms  
    - I/O read: 14.408 ms  
    - I/O write: N/A  
  
Shared buffers:  
  - hits: 85 (~680.00 KiB) from the buffer pool  
  - reads: 221 (~1.70 MiB) from the OS file cache, including disk I/O  
  - dirtied: 2 (~16.00 KiB)  
  - writes: 0  

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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 #331101 (closed)

Edited by Max Woolf

Merge request reports