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

038 (issue rebuild) migration performance improvement #806

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rpuch
Copy link

@rpuch rpuch commented Jan 22, 2013

platform: # supported: 2.2.4, 2.3.2
backlogs: # supported: 1.0.5
ruby: # supported: 1.9.3, 2.0.0

038 migration is pretty slow because in it a lot of work is made in individual INSERT/UPDATE queries. However, this migration seems to actually just add data to rb_sprint_burndown and rb_issue_history, so it's much faster to insert them in a bulk and don't do UPDATEs at all.

This is the main idea of the fix. To implement it, I had to evade calling issue.history everywhere because it momentarily makes an INSERT. Also, all calculations are made first (in memory), and only then data is inserted.

Besides this, some redundant SELECT and SHOW TABLES queries were removed.
It still can be improved, however (still too much queries), but it takes now 860 seconds on my DB versus more than 9000 seconds for initial implementation in v0.9.32 tag.

This approach has a possible downside: as associations are maintained manually, if Ruby code changes in the future, it can break this migration logic.

rpuch added 2 commits January 11, 2013 18:00
- using bulk inserts
- removed excessive selects
- using eager assosiations load to minimize query count
@patrickatamaniuk
Copy link
Contributor

Thats fast. Much appreciated.
It yields different results, tough. Every record has today appended. Calculation for remaining_hours in every first record differs - somtimes its another value, somtimes its emtpy. More records appear in the middle of some history entries.
If that is improvement or regression, i cannot verify at the moment.

Also, it requires rails-3. If we drop rails-2 support completely, is not decided yet. (+1 from me)
The code involves changes to the model api, @friflaj needs to review this. Please give us some time here.
The code has diverged since 0.9.32, this will not merge without conflict, currently.

In any case, thank you very much for your work, as this isssue is indeed relevant.

@rpuch
Copy link
Author

rpuch commented Jan 23, 2013

Oops... possibly I've missed something while merging.
Also, I tried to merge to the current master yesterday. Possibly I've made some mistake with git+github as I'm new to them. I'll try to fix this soon.

@rpuch
Copy link
Author

rpuch commented Jan 23, 2013

Removed my top commit and merged again, to the current master. I hope I don't mess this pull request up even worse :)
As for different results - it seems I've broken something, probably the custom_update_parent logic. It does not call parent recursively with non-nil date as update_parent did. I will try to sort this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants