Bug #10903

Cancel button on jobs & pipeline instances should cancel all child jobs too

Added by Tom Morris almost 3 years ago. Updated almost 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
01/17/2017
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0

Description

Extend the Job#cancel method to accept a "cascade: true" parameter.

Extend the jobs.cancel API to accept a "cascade=true" parameter.

Add a PipelineInstance#cancel method that accepts a "cascade: true" parameter (change to state=Paused if state is RunningOnServer or RunningOnClient; no-op if state is Paused; otherwise fail).

Add a pipeline_instances.cancel API that accepts a "cascade: true" parameter.

Update workbench to use the cascade=true flag when calling the jobs.cancel API.

Add a confirmation dialog which includes text warning that all unfinished child jobs will be canceled as well, even if they're also being used in another job/pipeline.

Update Workbench to use the pipeline_instances.cancel API with cascade=true, instead of updating state to Paused. Add a confirmation dialog as with jobs.

In the model methods, if cascade: true is given, cancel any jobs listed in the pipeline instance's / job's components hash (subject to permissions), and propagate the cascade:true flag so an arbitrary number of generations of descendants can be cancelled.

Ensure the code cannot fall into an infinite loop or recursion (for example, make a test case where job A has job B in its components hash, and job B has job A in its components hash).

This is crunch1 only. Crunch2 already takes care of this.


Subtasks

Task #11036: Review 10903-api-cancel-cascadeResolvedRadhika Chippada

Task #11073: Review 10903-wb-cancel-cascadeResolvedLucas Di Pentima

Associated revisions

Revision d6aab18f
Added by Radhika Chippada almost 3 years ago

closes #10903
Merge branch '10903-wb-cancel-cascade'

History

#1 Updated by Tom Morris almost 3 years ago

  • Story points set to 2.0
  • Description updated (diff)

#2 Updated by Tom Clegg almost 3 years ago

  • Description updated (diff)

#3 Updated by Tom Morris almost 3 years ago

  • Target version changed from 2017-02-01 sprint to 2017-02-15 sprint

#4 Updated by Tom Clegg almost 3 years ago

  • Subject changed from Cancel button should cancel all subjobs too to Cancel button on jobs & pipeline instances should cancel all child jobs too
  • Description updated (diff)

#5 Updated by Radhika Chippada almost 3 years ago

  • Assigned To set to Radhika Chippada

#6 Updated by Radhika Chippada almost 3 years ago

  • Status changed from New to In Progress

#7 Updated by Radhika Chippada almost 3 years ago

Branch 10903-api-cancel-cascade @ 3f833687 addresses the API server updates: (1) enhance job.cancel method to support cascade parameter, (2) add cancel method with cascade to pipeline_instance.

Tests @ https://ci.curoverse.com/job/developer-run-tests/157

#8 Updated by Radhika Chippada almost 3 years ago

Tom: branch 10903-wb-cancel-cascade @ 210af1cc7ede88914026fde078e45ef84c187a0c has both API and workbench updates. Please review this branch if you prefer to review both together.

One question: Let's say I have a job with job and pipeline components and I cancel it in workbench. Now I see that the main job and the child job are "cancelled", but the pipeline status is shown as "paused". If I were to navigate to that child pipeline, I would be able to click on it's resume button. Just wondering if this might be confusing from UI perspective.

API and workbench tests passed @ https://ci.curoverse.com/job/developer-run-tests/161/

Thanks.

#9 Updated by Tom Clegg almost 3 years ago

10903-api-cancel-cascade @ 3f83368

I think this needs to be protected by a transaction. Suggest new signature for cancel method:

def cancel(cascade: false, need_transaction: true)
  if need_transaction
    ActiveRecord::Base.transaction do
      cancel(cascade: cascade, need_transaction: false)
    end
    return
  end
  # ...

(ruby2 has native named params, so we should be using those instead of positional params...)

Can do more filtering in the database in these sections:

-    Job.where(uuid: children).each do |job|
-      job.cancel cascade if job.state.in?([Queued, Running])
+    Job.where(uuid: children, state: [Queued, Running]).each do |job|
+      job.cancel(cascade: cascade, need_transaction: false)
     end

#10 Updated by Radhika Chippada almost 3 years ago

Branch 10903-wb-cancel-cascade at 2e9cb97 addresses the suggestions from note 9 (need_transaction added).

Tests @ https://ci.curoverse.com/job/developer-run-tests/165/

#11 Updated by Lucas Di Pentima almost 3 years ago

LGTM!

#12 Updated by Radhika Chippada almost 3 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100

Applied in changeset arvados|commit:d6aab18f9688d46bc1f86f021d68e02e5601cfe7.

Also available in: Atom PDF