Merge pull request #1049 from Duke-Translational-Bioinformatics/develop

Merge v1.3.26 to ua_test

Author: dmlond

api_design/DDS-872-eventual_consistency_status.md

@@ -76,3 +76,16 @@ We will change the success response to [`202: Accepted`](https://httpstatuses.co
 ## Process View
 
 Add notes about performance, scalability, throughput, etc. here. These can inform future proposals to change the implementation.
+
+#### Deployment
+
+Environment Variable | Default | Description
+--- | --- | ---
+**ACTIVE_JOB_QUEUE_ADAPTER** | sneakers | this sets how activejob runs jobs. Jobs can be `inline` (jobs run immediately instead of in the background), or `sneakers` (jobs run in the background using rabbitmq)
+**CLOUDAMQP_URL** |  | this is the base url to the rabbitmq server. For heroku managed applications, this is set automatically when you provision the Cloud AMQP service. You can change this to point to another rabbitmq server. This is set in rabbitmq.client.env for the local docker-compose managed instance.
+**WORKERS_ALL_RUN_EXCEPT** |  |  comma-separated list of workers that are not meant to run. This is used by the `rake workers:all:run` task.
+
+**Notes**:
+When using the CloudAMQP plans that are not dedicated (little-lemur, tough-tiger), queues that are not accessed by workers for *Max idle queue time* (this setting can be viewed in the the CloudAMQP dashboard, accessed from heroku application dashboard using the browser) are deleted. Going forward we should consider the use of dedicated plans, and/or monitoring and alerts on job workers to notify us when jobs stop running.
+
+We use the heroku scheduler to run `rake queue:message_log:index_messages MESSAGE_LOG_WORK_DURATION=300` to drain the events in the message_log queue into an elasticsearch index. MESSAGE_LOG_WORK_DUR sets the length of time that this job runs. We may want to watch the message_log queue to ensure that it is being drained during each run of this task. If it is not, we can increase the work duration.

app/jobs/child_deletion_job.rb

@@ -1,12 +1,10 @@
 class ChildDeletionJob < ApplicationJob
   queue_as :child_deletion
 
-  def perform(job_transaction, parent)
+  def perform(job_transaction, parent, page)
     self.class.start_job(job_transaction)
-    parent.children.each do |child|
-      child.current_transaction = job_transaction if child.class.include? ChildMinder
-      child.update(is_deleted: true)
-    end
+    parent.current_transaction = job_transaction
+    parent.delete_children(page)
     self.class.complete_job(job_transaction)
   end
 end

app/models/concerns/child_minder.rb

@@ -9,20 +9,30 @@ def manage_children
     newly_deleted = is_deleted_changed? && is_deleted?
     yield
     if has_children? && newly_deleted
-      ChildDeletionJob.perform_later(
-        ChildDeletionJob.initialize_job(self),
-        self
-      )
+      (1..paginated_children.total_pages).each do |page|
+        ChildDeletionJob.perform_later(
+          ChildDeletionJob.initialize_job(self),
+          self,
+          page
+        )
+      end
     end
   end
 
-  def delete_children
-    children.each do |child|
+  def has_children?
+    children.count > 0
+  end
+
+  def delete_children(page)
+    paginated_children(page).each do |child|
+      child.current_transaction = current_transaction if child.class.include? ChildMinder
       child.update(is_deleted: true)
     end
   end
 
-  def has_children?
-    children.count > 0
+  private
+
+  def paginated_children(page=1)
+    children.page(page).per(Rails.application.config.max_children_per_job)
   end
 end

config/initializers/child_deletion_job.rb

@@ -0,0 +1 @@
+Rails.application.config.max_children_per_job = ENV['MAX_CHILDREN_PER_JOB'] ? ENV['MAX_CHILDREN_PER_JOB'].to_i : 100

spec/jobs/child_deletion_job_spec.rb

@@ -1,6 +1,48 @@
 require 'rails_helper'
 
 RSpec.describe ChildDeletionJob, type: :job do
+
+  shared_examples 'a ChildDeletionJob' do |
+      parent_sym,
+      child_folder_sym,
+      child_file_sym
+    |
+    let(:parent) { send(parent_sym) }
+    let(:job_transaction) { described_class.initialize_job(parent) }
+    let(:child_folder) { send(child_folder_sym) }
+    let(:child_file) { send(child_file_sym) }
+    let(:prefix) { Rails.application.config.active_job.queue_name_prefix }
+    let(:prefix_delimiter) { Rails.application.config.active_job.queue_name_delimiter }
+    include_context 'with job runner', described_class
+
+    it { is_expected.to be_an ApplicationJob }
+    it { expect(prefix).not_to be_nil }
+    it { expect(prefix_delimiter).not_to be_nil }
+    it { expect(described_class.queue_name).to eq("#{prefix}#{prefix_delimiter}child_deletion") }
+    it {
+      expect {
+        described_class.perform_now
+      }.to raise_error(ArgumentError)
+      expect {
+        described_class.perform_now(job_transaction)
+      }.to raise_error(ArgumentError)
+      expect {
+        described_class.perform_now(job_transaction, parent)
+      }.to raise_error(ArgumentError)
+    }
+
+    context 'perform_now' do
+      let(:page) { 1 }
+
+      it {
+        expect(child_folder).to be_persisted
+        expect(child_file).to be_persisted
+        expect(parent).to receive(:delete_children).with(page)
+        described_class.perform_now(job_transaction, parent, page)
+      }
+    end
+  end
+
   before do
     expect(folder_child).to be_persisted
   end

spec/support/concerns/child_minder_shared_examples.rb

@@ -65,6 +65,8 @@
           ChildDeletionJob.initialize_job(subject)
         }
         before do
+          @old_max = Rails.application.config.max_children_per_job
+          Rails.application.config.max_children_per_job = 1
           expect(child_folder).to be_persisted
           expect(child_folder.is_deleted?).to be_falsey
           expect(valid_child_file).to be_persisted
@@ -73,13 +75,19 @@
           expect(invalid_child_file.is_deleted?).to be_falsey
         end
 
+        after do
+          Rails.application.config.max_children_per_job = @old_max
+        end
+
         it {
           expect(subject).to be_has_children
           subject.is_deleted = true
           yield_called = false
-          expect(ChildDeletionJob).to receive(:initialize_job)
-            .with(subject).and_return(job_transaction)
-          expect(ChildDeletionJob).to receive(:perform_later).with(job_transaction, subject)
+          expect(ChildDeletionJob).to receive(:initialize_job).exactly(subject.children.count)
+            .with(subject).times.and_return(job_transaction)
+          (1..subject.children.count).each do |page|
+            expect(ChildDeletionJob).to receive(:perform_later).with(job_transaction, subject, page)
+          end
           subject.manage_children do
             yield_called = true
           end
@@ -133,23 +141,41 @@
   end
 
   describe '#delete_children' do
-    it {
-      is_expected.to respond_to(:delete_children)
-    }
-    it {
-      expect(child_folder).to be_persisted
-      expect(child_folder.is_deleted?).to be_falsey
-      expect(valid_child_file).to be_persisted
-      expect(valid_child_file.is_deleted?).to be_falsey
-      expect(invalid_child_file).to be_persisted
-      expect(invalid_child_file.is_deleted?).to be_falsey
-      subject.delete_children
-      expect(child_folder.reload).to be_truthy
-      expect(child_folder.is_deleted?).to be_truthy
-      valid_child_file.reload
-      expect(valid_child_file.is_deleted?).to be_truthy
-      invalid_child_file.reload
-      expect(invalid_child_file.is_deleted?).to be_truthy
-    }
+    it { is_expected.not_to respond_to(:delete_children).with(0).arguments }
+    it { is_expected.to respond_to(:delete_children).with(1).argument }
+
+    context 'called', :vcr do
+      let(:job_transaction) { ChildDeletionJob.initialize_job(subject) }
+      let(:child_job_transaction) { ChildDeletionJob.initialize_job(child_folder) }
+      let(:child_folder_file) { FactoryGirl.create(:data_file, parent: child_folder)}
+      let(:page) { 1 }
+
+      before do
+        expect(child_folder).to be_persisted
+        expect(child_folder_file).to be_persisted
+        expect(valid_child_file.is_deleted?).to be_falsey
+        @old_max = Rails.application.config.max_children_per_job
+        Rails.application.config.max_children_per_job = subject.children.count + child_folder.children.count
+      end
+
+      after do
+        Rails.application.config.max_children_per_job = @old_max
+      end
+
+      it {
+        subject.current_transaction = job_transaction
+        expect(ChildDeletionJob).to receive(:initialize_job)
+          .with(child_folder)
+          .and_return(child_job_transaction)
+        expect(ChildDeletionJob).to receive(:perform_later)
+          .with(child_job_transaction, child_folder, page).and_call_original
+        subject.delete_children(page)
+        expect(child_folder.reload).to be_truthy
+        expect(child_folder.is_deleted?).to be_truthy
+        expect(valid_child_file.reload).to be_truthy
+        expect(valid_child_file.is_deleted?).to be_truthy
+      }
+    end
+
   end
-end
+ end

spec/support/job_shared_examples.rb

@@ -163,52 +163,3 @@
       .and_call_original
   end
 end
-
-shared_examples 'a ChildDeletionJob' do |
-    parent_sym,
-    child_folder_sym,
-    child_file_sym
-  |
-  let(:parent) { send(parent_sym) }
-  let(:job_transaction) { described_class.initialize_job(parent) }
-  let(:child_folder) { send(child_folder_sym) }
-  let(:child_file) { send(child_file_sym) }
-  let(:prefix) { Rails.application.config.active_job.queue_name_prefix }
-  let(:prefix_delimiter) { Rails.application.config.active_job.queue_name_delimiter }
-  include_context 'with job runner', described_class
-
-  it { is_expected.to be_an ApplicationJob }
-  it { expect(prefix).not_to be_nil }
-  it { expect(prefix_delimiter).not_to be_nil }
-  it { expect(described_class.queue_name).to eq("#{prefix}#{prefix_delimiter}child_deletion") }
-  it {
-    expect {
-      described_class.perform_now
-    }.to raise_error(ArgumentError)
-    expect {
-      described_class.perform_now(parent)
-    }.to raise_error(ArgumentError)
-  }
-
-  context 'perform_now', :vcr do
-    let(:child_job_transaction) { described_class.initialize_job(child_folder) }
-    include_context 'tracking job', :job_transaction
-
-    it {
-      expect(child_folder).to be_persisted
-      expect(child_file.is_deleted?).to be_falsey
-
-      expect(described_class).to receive(:initialize_job)
-        .with(child_folder)
-        .and_return(child_job_transaction)
-      expect(described_class).to receive(:perform_later)
-        .with(child_job_transaction, child_folder).and_call_original
-
-      described_class.perform_now(job_transaction, parent)
-      expect(child_folder.reload).to be_truthy
-      expect(child_folder.is_deleted?).to be_truthy
-      expect(child_file.reload).to be_truthy
-      expect(child_file.is_deleted?).to be_truthy
-    }
-  end
-end