Project

General

Profile

Actions

Bug #10903

closed

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

Added by Tom Morris almost 8 years ago. Updated almost 8 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 2 (0 open2 closed)

Task #11036: Review 10903-api-cancel-cascadeResolvedRadhika Chippada01/17/2017

Actions
Task #11073: Review 10903-wb-cancel-cascadeResolvedLucas Di Pentima02/08/2017

Actions
Actions #1

Updated by Tom Morris almost 8 years ago

  • Story points set to 2.0
  • Description updated (diff)
Actions #2

Updated by Tom Clegg almost 8 years ago

  • Description updated (diff)
Actions #3

Updated by Tom Morris almost 8 years ago

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

Updated by Tom Clegg almost 8 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)
Actions #5

Updated by Radhika Chippada almost 8 years ago

  • Assigned To set to Radhika Chippada
Actions #6

Updated by Radhika Chippada almost 8 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Radhika Chippada almost 8 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

Actions #8

Updated by Radhika Chippada almost 8 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.

Actions #9

Updated by Tom Clegg almost 8 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
Actions #10

Updated by Radhika Chippada almost 8 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/

Actions #11

Updated by Lucas Di Pentima almost 8 years ago

LGTM!

Actions #12

Updated by Radhika Chippada almost 8 years ago

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

Applied in changeset arvados|commit:d6aab18f9688d46bc1f86f021d68e02e5601cfe7.

Actions

Also available in: Atom PDF