Skip to content

Commit

Permalink
Base JobBatch#complete? on the presence of completed_at.
Browse files Browse the repository at this point in the history
The old logic was designed to figure out when to mark the
job batch as complete by setting the timestamp -- but
that has been moved into the lua script. Basing `complete?`
on the job jid sets is potentially risky since we have
discussed making some of the job batch redis keys expire
sooner than others. If the job jid set keys vanished from
redis the old logic would wrongly report the job batch
was not complete even though it was. The new logic
relies only on the `meta` key which is the primary key
used by a job batch to track its state.
  • Loading branch information
myronmarston committed Jan 3, 2014
1 parent f032f2e commit 17ea826
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 17 deletions.
6 changes: 1 addition & 5 deletions lib/plines/job_batch.rb
Expand Up @@ -132,7 +132,7 @@ def complete_job(qless_job)
end

def complete?
_complete?(pending_job_jids.length, completed_job_jids.length)
!!completed_at
end

def resolve_external_dependency(dep_name)
Expand Down Expand Up @@ -319,10 +319,6 @@ def update_external_dependency(dep_name, meth, jids)
end
end

def _complete?(pending_size, complete_size)
pending_size == 0 && complete_size > 0
end

def time_from(meta_entry)
date_string = meta[meta_entry]
Time.iso8601(date_string) if date_string
Expand Down
17 changes: 5 additions & 12 deletions spec/unit/plines/job_batch_spec.rb
Expand Up @@ -501,23 +501,16 @@ def create_batch_with_job
end

describe "#complete?" do
it 'returns false when there are no pending or completed jobs' do
it 'returns true if the job batch has a completed_at timestamp' do
batch = JobBatch.create(qless, pipeline_module, "foo", {})
expect(batch).not_to be_complete
batch.meta['completed_at'] = Time.now.getutc.iso8601
expect(batch).to be_complete
end

it 'returns false when there are pending jobs and completed jobs' do
it 'returns false if it lacks a completed_at timestamp' do
batch = JobBatch.create(qless, pipeline_module, "foo", {})
batch.pending_job_jids << "a"
batch.completed_job_jids << "b"
expect(batch).not_to be_complete
end

it 'returns true when there are only completed jobs' do
batch = JobBatch.create(qless, pipeline_module, "foo", {})
batch.completed_job_jids << "b"
expect(batch).to be_complete
end
end

describe "#pending_qless_jobs" do
Expand Down Expand Up @@ -742,7 +735,7 @@ def update_dependency(batch, name)
end

it 'returns true for a complete job batch' do
batch.completed_job_jids << "a"
batch.meta['completed_at'] = Time.now.getutc.iso8601
expect(batch.in_terminal_state?).to be_true
end

Expand Down

0 comments on commit 17ea826

Please sign in to comment.