Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Don't load all records into memory in the import rake task #471

Closed
wants to merge 1 commit into from

2 participants

@alan

Since #381 the rake task to bulk import all the records loads all the records into memory to count the number of records. The change was done because what looks like what was a bug in ActiveRecord when using default_scope and sql grouping.

I'd like to get that change reverted back, otherwise there is no point on paginating the records from the DB since the all the records have been previously loaded into memory. I've also tried to replicate the default_scope bug with ActiveRecord v3.2.8 (latest released version) and it looks like the issue with count and grouping has been fixed.

We'll be soon doing a bulk import and the current task with this behaviour of loading all the records into memory is not practical

@karmi
Owner

Hi, yes, makes sense -- will it work with other storage engines such as MongoDB etc?

@karmi karmi was assigned
@alan

I don't have a setup with mongoDB and Tire, but I suspect that as long as the mongo client implements ActiveModel it should work.

I've tested Model.count with a Mongoid Model and it works so I see no reason why it wouldn't work.

@alan

Any updates on this PR @karmi ? I believe Mongo based ORM's should keep working as expected. Let me know if you want me to check anything else to get this merged in.

@karmi
Owner

Hi, will test it agains a Mongoid based app, so we don't break it for them. Should get to it this week.

@karmi karmi closed this pull request from a commit
@alan alan [FIX] Use `MyModel.count`, not `MyModel.all.count` in the Rake import…
… task

Previously, all records had to be loaded into memory (errr),
better to use the `Model.count` method

Closes #471
879a048
@karmi karmi closed this in 879a048
@karmi
Owner

Thanks!, verified, merged, pushed.

@NOX73 NOX73 referenced this pull request from a commit in NOX73/tire
@alan alan [FIX] Use `MyModel.count`, not `MyModel.all.count` in the Rake import…
… task

Previously, all records had to be loaded into memory (errr),
better to use the `Model.count` method

Closes #471
027ad53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  lib/tire/tasks.rb
View
2  lib/tire/tasks.rb
@@ -60,7 +60,7 @@ def elapsed_to_human(elapsed)
STDOUT.sync = true
puts "[IMPORT] Starting import for the '#{ENV['CLASS']}' class"
tty_cols = 80
- total = klass.all.count rescue nil
+ total = klass.count rescue nil
offset = (total.to_s.size*2)+8
done = 0
Something went wrong with that request. Please try again.