这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@tirumaraiselvan
Copy link
Contributor

@tirumaraiselvan tirumaraiselvan commented Oct 25, 2019

Description

Improve the fetch new events query for Event Triggers. This should significantly reduce the base line load on PG.

Affected components

  • Server
  • Console

Solution and Design

  1. When an event trigger is deleted, set archived = 't' for all its corresponding events. Instead of joining to find active event triggers (like it happens currently), check for archived.

  2. Add index to delivered column.

Steps to test and verify

postgres=# explain  SELECT l.id
                        FROM hdb_catalog.event_log l
                        JOIN hdb_catalog.event_triggers e
                        ON (l.trigger_name = e.name)
                        WHERE l.delivered ='f' and l.error = 'f' and l.locked = 'f'
                              and (l.next_retry_at is NULL or l.next_retry_at <= now())
                        FOR UPDATE SKIP LOCKED
                        LIMIT 100 ;
                                                                QUERY PLAN                                                                
------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.57..30.06 rows=100 width=49)
   ->  LockRows  (cost=0.57..64027.16 rows=217110 width=49)
         ->  Merge Join  (cost=0.57..61856.06 rows=217110 width=49)
               Merge Cond: (l.trigger_name = e.name)
               ->  Index Scan using event_log_trigger_name_idx on event_log l  (cost=0.42..59087.56 rows=217110 width=56)
                     Filter: ((NOT delivered) AND (NOT error) AND (NOT locked) AND ((next_retry_at IS NULL) OR (next_retry_at <= now())))
               ->  Index Scan using event_triggers_pkey on event_triggers e  (cost=0.15..53.70 rows=370 width=38)
(7 rows)

postgres=# explain  SELECT l.id
                        FROM (select id, trigger_name from hdb_catalog.event_log 
                                      WHERE delivered ='f' and error = 'f' and locked = 'f'
                              and (next_retry_at is NULL or next_retry_at <= now()) and archived = 'f') l
                        FOR UPDATE SKIP LOCKED
                        LIMIT 100;
                                                                      QUERY PLAN                                                                       
-------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.42..16.54 rows=100 width=43)
   ->  LockRows  (cost=0.42..34995.02 rows=217110 width=43)
         ->  Index Scan using event_log_delivered_idx on event_log  (cost=0.42..32823.92 rows=217110 width=43)
               Index Cond: (delivered = false)
               Filter: ((NOT delivered) AND (NOT error) AND (NOT locked) AND (NOT archived) AND ((next_retry_at IS NULL) OR (next_retry_at <= now())))
(5 rows)

Limitations, known bugs & workarounds

It's not clear if the index would help because sometimes postgres prefers seq scan (during experiments), in which case it is just costly to maintain the index.

@netlify
Copy link

netlify bot commented Oct 25, 2019

Deploy preview for hasura-docs ready!

Built with commit d0737a1

https://deploy-preview-3236--hasura-docs.netlify.com

@arvi3411301
Copy link
Member

/heroku deploy

@hasura-bot
Copy link
Contributor

Review app for commit ca07436 deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-ca07436a

Copy link
Contributor

@lexi-lambda lexi-lambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks good to me, aside from one small comment. Do you think it would be possible to add a benchmark for this? Or would it be a lot of effort?

@tirumaraiselvan
Copy link
Contributor Author

@wawhal @rikinsk Can we accommodate archived column in the events console page?

lexi-lambda
lexi-lambda previously approved these changes Oct 29, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 308bfcf deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-308bfcf0

@tirumaraiselvan tirumaraiselvan added the s/do-not-merge Do not merge this pull request to master label Oct 29, 2019
@hasura-bot
Copy link
Contributor

Review app for commit f6ad1a5 deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-f6ad1a52

@tirumaraiselvan tirumaraiselvan force-pushed the improve_event_fetch_query branch from f6ad1a5 to 308bfcf Compare October 29, 2019 13:18
@hasura-bot
Copy link
Contributor

Review app for commit 0be28a5 deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-0be28a51

@tirumaraiselvan tirumaraiselvan removed the s/do-not-merge Do not merge this pull request to master label Nov 8, 2019
@tirumaraiselvan
Copy link
Contributor Author

@rikinsk I tested this and seems fine.

lexi-lambda
lexi-lambda previously approved these changes Nov 12, 2019
@hasura-bot
Copy link
Contributor

Review app for commit e0f7e19 deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-e0f7e199

@shahidhk shahidhk requested a review from lexi-lambda November 13, 2019 06:34
@hasura-bot
Copy link
Contributor

Review app for commit d0737a1 deployed to Heroku: https://hge-ci-pull-3236.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3236-d0737a1b

@shahidhk shahidhk changed the title Improve event fetch query improve event fetch query Nov 13, 2019
@shahidhk shahidhk merged commit 3cad131 into hasura:master Nov 13, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-3236.herokuapp.com is deleted

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants