Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: jobs table index by default #27

Closed
bnauta opened this issue Sep 13, 2018 · 4 comments
Closed

Performance: jobs table index by default #27

bnauta opened this issue Sep 13, 2018 · 4 comments

Comments

@bnauta
Copy link

bnauta commented Sep 13, 2018

We're currently using Rihanna in production and ran into a major issue where Job.lock was taking nearly 3 minutes to execute on ~160k records. The vast majority of performance hit can be blamed on these comparison and sort operations, which, conveniently, can be addressed by a single index.

CREATE INDEX CONCURRENTLY rihanna_jobs_enqueued_at_id ON rihanna_jobs (enqueued_at ASC, id ASC);

Job.lock now runs on the same dataset at < 2ms.

I've forked and fleshed out the benchmark with a test for 100k records and the above index, but ultimately would like to include the index by default on table create/update.

Creating this issue to track progress and discussion.

@samsondav
Copy link
Owner

@bnauta Amazing work! I can't believe I forgot the index...

I actually had this locally because I added it manually during my testing, but clean forgot to add it to the Rihanna SQL. If you like, would you be able to open a PR that includes an upgrade to add the index as well as adding it to the main create SQL?

@samsondav
Copy link
Owner

Fixed in #28

@bnauta
Copy link
Author

bnauta commented Sep 14, 2018

@samphilipd thanks for handling this so quickly! I've been a little swamped.

I noted that CREATE INDEX ... IF NOT EXISTS is only supported in PostgreSQL 9.5+, and it seems we're aiming to support older versions.

We can ensure backwards compatibility by using an approach such as this (see: v9.3 and older solution). A little lower priority, but I could create a PR in the next few days.

@samsondav
Copy link
Owner

samsondav commented Sep 14, 2018

Rihanna officially supports postgres 9.5+ so I think this is fine. Thanks for looking into it though!

EDIT: I forgot to add, the reason we require at least 9.5 is because we use FOR UPDATE SKIP LOCKED which is only available on these versions anyway.

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

No branches or pull requests

2 participants