Commit 0a38626f0a32cc17f9c9b6f1af36632d66196653
1 parent
33c616dd
Exists in
master
and in
1 other branch
catchup fix
when an idea is activated, reduce prompt_queue cache to refill size and schedule delayed job to regenerate prompts
Showing
3 changed files
with
30 additions
and
3 deletions
Show diff stats
app/models/choice.rb
| @@ -16,6 +16,7 @@ class Choice < ActiveRecord::Base | @@ -16,6 +16,7 @@ class Choice < ActiveRecord::Base | ||
| 16 | named_scope :inactive, :conditions => { :active => false} | 16 | named_scope :inactive, :conditions => { :active => false} |
| 17 | 17 | ||
| 18 | after_save :update_questions_counter | 18 | after_save :update_questions_counter |
| 19 | + after_save :update_prompt_queue | ||
| 19 | 20 | ||
| 20 | attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count | 21 | attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count |
| 21 | attr_readonly :question_id | 22 | attr_readonly :question_id |
| @@ -23,6 +24,14 @@ class Choice < ActiveRecord::Base | @@ -23,6 +24,14 @@ class Choice < ActiveRecord::Base | ||
| 23 | def update_questions_counter | 24 | def update_questions_counter |
| 24 | self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) | 25 | self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) |
| 25 | end | 26 | end |
| 27 | + | ||
| 28 | + # if changing a choice to active, we want to regenerate prompts | ||
| 29 | + def update_prompt_queue | ||
| 30 | + if self.changed.include?('active') && self.active? | ||
| 31 | + self.question.mark_prompt_queue_for_refill | ||
| 32 | + self.question.choose_prompt | ||
| 33 | + end | ||
| 34 | + end | ||
| 26 | 35 | ||
| 27 | def before_create | 36 | def before_create |
| 28 | unless self.score | 37 | unless self.score |
app/models/question.rb
| @@ -29,6 +29,12 @@ class Question < ActiveRecord::Base | @@ -29,6 +29,12 @@ class Question < ActiveRecord::Base | ||
| 29 | 29 | ||
| 30 | attr_readonly :site_id | 30 | attr_readonly :site_id |
| 31 | 31 | ||
| 32 | + # regenerate prompts if cache is less than this full | ||
| 33 | + # ideally this prevents active marketplaces from | ||
| 34 | + # regenerating prompts too frequently | ||
| 35 | + @@percent_full = 0.9 | ||
| 36 | + @@num_prompts = 1000 | ||
| 37 | + | ||
| 32 | named_scope :created_by, lambda { |id| | 38 | named_scope :created_by, lambda { |id| |
| 33 | {:conditions => { :local_identifier => id } } | 39 | {:conditions => { :local_identifier => id } } |
| 34 | } | 40 | } |
| @@ -474,13 +480,20 @@ class Question < ActiveRecord::Base | @@ -474,13 +480,20 @@ class Question < ActiveRecord::Base | ||
| 474 | $redis.del(self.pq_key) | 480 | $redis.del(self.pq_key) |
| 475 | end | 481 | end |
| 476 | 482 | ||
| 483 | + | ||
| 484 | + # make prompt queue less than @@precent_full | ||
| 485 | + def mark_prompt_queue_for_refill | ||
| 486 | + # 2 because redis starts indexes at 0 | ||
| 487 | + new_size = (@@num_prompts * @@percent_full - 2).floor | ||
| 488 | + $redis.ltrim(self.pq_key, 0, new_size) | ||
| 489 | + end | ||
| 490 | + | ||
| 477 | def add_prompt_to_queue | 491 | def add_prompt_to_queue |
| 478 | - num_prompts = 1000 | ||
| 479 | # if less than 90% full, regenerate prompts | 492 | # if less than 90% full, regenerate prompts |
| 480 | # we skip generating prompts if more than 90% full to | 493 | # we skip generating prompts if more than 90% full to |
| 481 | # prevent one busy marketplace for ruling the queue | 494 | # prevent one busy marketplace for ruling the queue |
| 482 | - if $redis.llen(self.pq_key) < num_prompts * 0.9 | ||
| 483 | - prompts = self.catchup_choose_prompt(num_prompts) | 495 | + if $redis.llen(self.pq_key) < @@num_prompts * @@percent_full |
| 496 | + prompts = self.catchup_choose_prompt(@@num_prompts) | ||
| 484 | # clear list | 497 | # clear list |
| 485 | $redis.ltrim(self.pq_key, 0, 0) | 498 | $redis.ltrim(self.pq_key, 0, 0) |
| 486 | $redis.lpop(self.pq_key) | 499 | $redis.lpop(self.pq_key) |
spec/models/choice_spec.rb
| @@ -86,6 +86,11 @@ describe Choice do | @@ -86,6 +86,11 @@ describe Choice do | ||
| 86 | choice1.should be_active | 86 | choice1.should be_active |
| 87 | end | 87 | end |
| 88 | 88 | ||
| 89 | + it "should create a delayed job on activation" do | ||
| 90 | + choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | ||
| 91 | + proc { choice1.deactivate! }.should change(Delayed::Job, :count).by(1) | ||
| 92 | + end | ||
| 93 | + | ||
| 89 | it "should update a question's counter cache on deactivation" do | 94 | it "should update a question's counter cache on deactivation" do |
| 90 | prev_inactive = @question.inactive_choices_count | 95 | prev_inactive = @question.inactive_choices_count |
| 91 | choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | 96 | choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) |