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

clean bulk upsert optimization #89

Merged
merged 3 commits into from
Nov 9, 2016
Merged

clean bulk upsert optimization #89

merged 3 commits into from
Nov 9, 2016

Conversation

pallavi2209
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 84.122% when pulling 3bbd18c on clean-bulk-upsert into 517b715 on master.

queryInterface.addIndex(
'Samples',
['name'],
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to remove this? Seems like a good thing to have around regardless, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would hurt us to have extra indexes.
See: http://stackoverflow.com/questions/20707182/disadvantages-of-creating-multiple-indexes-in-a-postgresql-table

indexes have a performance penalty when it comes to INSERTing new rows, DELETEing old ones or UPDATEing existing values of the indexed column, as now the DML statement need not only modify the table's data, but the index's one too

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

(... although I could argue that with our usage pattern there wouldn't actually be a penalty for indexing name because samples rarely get created or deleted, and when they're updated, the name is typically not updated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.. I see your point. I can add the index back in if @shriramshankar has no issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://devcenter.heroku.com/articles/expensive-queries. Looks like unused indexes could also be a cause of slow DML operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok get rid of it

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 84.034% when pulling d9e479b on clean-bulk-upsert into 893f46a on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 83.959% when pulling ffb2958 on clean-bulk-upsert into 1982d90 on master.

@iamigo iamigo merged commit d27f26d into master Nov 9, 2016
@iamigo iamigo deleted the clean-bulk-upsert branch November 9, 2016 18:49
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.

4 participants