Changed async reindex to fetch ids instead of using ranges for numeric primary k…

…eys with Active Record [skip ci]

Changed async reindex to fetch ids instead of using ranges for numeric primary k…
…eys with Active Record [skip ci]
Andrew Kane
1 parent 64fbf6e5
Showing 2 changed files with 8 additions and 30 deletions Show diff stats
CHANGELOG.md
lib/searchkick/relation_indexer.rb
@@ -4,6 +4,7 @@
 - Added support for `:async` and `:queue` modes for `reindex` on relation
 - Added basic protection from unfiltered parameters to `where` option
 - Added `models` option to `similar` method
+- Changed async reindex to fetch ids instead of using ranges for numeric primary keys with Active Record
 - Anchor regular expressions by default
 - Raise error when `search` called on relations
 - Raise `ArgumentError` (instead of warning) for invalid regular expression modifiers
@@ -72,37 +72,14 @@ module Searchkick
       batch_id = 1
       class_name = relation.searchkick_options[:class_name]
  
-      if relation.respond_to?(:primary_key)
-        # TODO expire Redis key
-        primary_key = relation.primary_key
-
-        starting_id =
-          begin
-            relation.minimum(primary_key)
-          rescue ActiveRecord::StatementInvalid
-            false
-          end
-
-        if starting_id.nil?
-          # no records, do nothing
-        elsif starting_id.is_a?(Numeric)
-          max_id = relation.maximum(primary_key)
-          batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
-
-          batches_count.times do |i|
-            min_id = starting_id + (i * batch_size)
-            batch_job(class_name, batch_id, min_id: min_id, max_id: min_id + batch_size - 1)
-            batch_id += 1
-          end
-        else
-          relation.find_in_batches(batch_size: batch_size) do |batch|
-            batch_job(class_name, batch_id, record_ids: batch.map { |record| record.id.to_s })
-            batch_id += 1
-          end
+      if relation.respond_to?(:find_in_batches)
+        relation.find_in_batches(batch_size: batch_size) do |items|
+          batch_job(class_name, batch_id, items.map(&:id))
+          batch_id += 1
         end
       else
         each_batch(relation, batch_size: batch_size) do |items|
-          batch_job(class_name, batch_id, record_ids: items.map { |i| i.id.to_s })
+          batch_job(class_name, batch_id, items.map(&:id))
           batch_id += 1
         end
       end
@@ -122,13 +99,13 @@ module Searchkick
       yield items if items.any?
     end
  
-    def batch_job(class_name, batch_id, **options)
+    def batch_job(class_name, batch_id, record_ids)
       Searchkick.with_redis { |r| r.sadd(batches_key, batch_id) }
       Searchkick::BulkReindexJob.perform_later(
         class_name: class_name,
         index_name: index.name,
         batch_id: batch_id,
-        **options
+        record_ids: record_ids.map { |v| v.instance_of?(Integer) ? v : v.to_s }
       )
     end
...	...	@@ -4,6 +4,7 @@
4	4	- Added support for `:async` and `:queue` modes for `reindex` on relation
5	5	- Added basic protection from unfiltered parameters to `where` option
6	6	- Added `models` option to `similar` method
	7	+- Changed async reindex to fetch ids instead of using ranges for numeric primary keys with Active Record
7	8	- Anchor regular expressions by default
8	9	- Raise error when `search` called on relations
9	10	- Raise `ArgumentError` (instead of warning) for invalid regular expression modifiers
...	...
...	...	@@ -72,37 +72,14 @@ module Searchkick
72	72	batch_id = 1
73	73	class_name = relation.searchkick_options[:class_name]
74	74
75		- if relation.respond_to?(:primary_key)
76		- # TODO expire Redis key
77		- primary_key = relation.primary_key
78		-
79		- starting_id =
80		- begin
81		- relation.minimum(primary_key)
82		- rescue ActiveRecord::StatementInvalid
83		- false
84		- end
85		-
86		- if starting_id.nil?
87		- # no records, do nothing
88		- elsif starting_id.is_a?(Numeric)
89		- max_id = relation.maximum(primary_key)
90		- batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
91		-
92		- batches_count.times do \|i\|
93		- min_id = starting_id + (i * batch_size)
94		- batch_job(class_name, batch_id, min_id: min_id, max_id: min_id + batch_size - 1)
95		- batch_id += 1
96		- end
97		- else
98		- relation.find_in_batches(batch_size: batch_size) do \|batch\|
99		- batch_job(class_name, batch_id, record_ids: batch.map { \|record\| record.id.to_s })
100		- batch_id += 1
101		- end
	75	+ if relation.respond_to?(:find_in_batches)
	76	+ relation.find_in_batches(batch_size: batch_size) do \|items\|
	77	+ batch_job(class_name, batch_id, items.map(&:id))
	78	+ batch_id += 1
102	79	end
103	80	else
104	81	each_batch(relation, batch_size: batch_size) do \|items\|
105		- batch_job(class_name, batch_id, record_ids: items.map { \|i\| i.id.to_s })
	82	+ batch_job(class_name, batch_id, items.map(&:id))
106	83	batch_id += 1
107	84	end
108	85	end
...	...	@@ -122,13 +99,13 @@ module Searchkick
122	99	yield items if items.any?
123	100	end
124	101
125		- def batch_job(class_name, batch_id, **options)
	102	+ def batch_job(class_name, batch_id, record_ids)
126	103	Searchkick.with_redis { \|r\| r.sadd(batches_key, batch_id) }
127	104	Searchkick::BulkReindexJob.perform_later(
128	105	class_name: class_name,
129	106	index_name: index.name,
130	107	batch_id: batch_id,
131		- **options
	108	+ record_ids: record_ids.map { \|v\| v.instance_of?(Integer) ? v : v.to_s }
132	109	)
133	110	end
134	111
...	...