Project

General

Profile

Actions
Copy link

Bug #13681

closed

[CWL] Cannot set submit-runner-ram when running from Workbench

Added by Peter Amstutz over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Peter Amstutz
Category:
-
Target version:
2018-07-18 Sprint
Story points:
1.0
Release:
Arvados 1.2
Release relationship:
Auto

Description

Workbench has the RAM allocation for arvados-cwl-runner jobs hardcoded at 256MB. This is way too stingy (when you run from the command line, the default is 1GB) and users with large collection inputs may need as much as 8GB. When running from workbench, there is no way to set --submit-runner-ram. This should be settable, at minimum when using a-c-r --create-workflow.

Proposed fix:

  • Add a "workflow_runner_ram" column to the Workflows table
  • When using "a-c-r --create-workflow" or "a-c-r --update-workflow", looks for "--submit-runner-ram" and set "workflow_runner_ram" in the request
  • When workbench constructs a container request to run a-c-r, use the value of "workflow_runner_ram" for "--submit-runner-ram" on the a-c-r command line.

Subtasks 4 (0 open — 4 closed)

Task #13721: a-c-r understands runner ram hintResolvedPeter Amstutz07/05/2018Actions
Task #13722: Workbench understands runner ram hintResolvedPeter Amstutz07/05/2018Actions
Task #13723: Review 13681-wb-acr-submit-ramResolvedPeter Amstutz07/05/2018Actions
Task #13724: Document runner ram hintResolvedPeter Amstutz07/05/2018Actions

Related issues 1 (1 open — 0 closed)

Related to Arvados - Bug #13766: [API] bug in libyaml / Pysch used by API server to parse yamlIn ProgressPeter AmstutzActions
Actions
Copy link
 #1

Updated by Peter Amstutz over 6 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #2

Updated by Peter Amstutz over 6 years ago

  • Description updated (diff)
Actions
Copy link
 #3

Updated by Tom Morris over 6 years ago

  • Release set to 13
Actions
Copy link
 #4

Updated by Peter Amstutz over 6 years ago

  • Description updated (diff)
  • Status changed from In Progress to New
  • Release deleted (13)
Actions
Copy link
 #5

Updated by Peter Amstutz over 6 years ago

  • Release set to 14
Actions
Copy link
 #6

Updated by Peter Amstutz over 6 years ago

  • Release changed from 14 to 13
Actions
Copy link
 #7

Updated by Tom Clegg over 6 years ago

Is there an appropriate place to put this information in the workflow definition itself, rather than a separate column?

Actions
Copy link
 #8

Updated by Peter Amstutz over 6 years ago

Tom Clegg wrote:

Is there an appropriate place to put this information in the workflow definition itself, rather than a separate column?

Yes, I suppose we could add a new hint for it. Workbench would still need to know how to extract it in order to construct the container request.

Actions
Copy link
 #9

Updated by Tom Morris over 6 years ago

  • Target version changed from To Be Groomed to 2018-07-18 Sprint
  • Story points set to 1.0
Actions
Copy link
 #10

Updated by Peter Amstutz over 6 years ago

  • Assigned To set to Peter Amstutz
Actions
Copy link
 #11

Updated by Peter Amstutz over 6 years ago

13681-wb-acr-submit-ram @ 4e714090c2c81cddcef6da393b5680f3fcfe40b1

https://ci.curoverse.com/job/developer-run-tests/784/

  • New hint arv:WorkflowRunnerResources
  • Recognized by a-c-r Runner class and Workbench to set runtime_constraints for "ram" and "vcpus"
  • Providing --submit-runner-ram with --create/update-workflow will implicitly set arv:WorkflowRunnerResources in hints
  • Updated documentation
Actions
Copy link
 #12

Updated by Lucas Di Pentima over 6 years ago

Reviewing 334c069f4da11278844f4276b5c254e8cbf06412

  • There’re some sdk/cwl test failures on jenkins (already talked about this, I'm just recording the observation)
  • Just some code style observations on file sdk/cwl/arvados_cwl/arvworkflow.py
    • Line 55: Could that be replace with this?: hints = main.get(‘hints’, [])
    • The found var on line 57 could be replaced with an else clause on the for loop?
Actions
Copy link
 #13

Updated by Peter Amstutz over 6 years ago

Lucas Di Pentima wrote:

Reviewing 334c069f4da11278844f4276b5c254e8cbf06412

  • There’re some sdk/cwl test failures on jenkins (already talked about this, I'm just recording the observation)

Yea, I'm going to revert the change which is causing those failures, I filed #13766 to dig deeper.

  • Just some code style observations on file sdk/cwl/arvados_cwl/arvworkflow.py
    • Line 55: Could that be replace with this?: hints = main.get(‘hints’, [])

Yes, I don't know why I wrote it that way.

  • The found var on line 57 could be replaced with an else clause on the for loop?

No, because (according to my research) the "else" clause doesn't fire for empty arrays.

Actions
Copy link
 #15

Updated by Peter Amstutz over 6 years ago

  • Related to Bug #13766: [API] bug in libyaml / Pysch used by API server to parse yaml added
Actions
Copy link
 #16

Updated by Lucas Di Pentima over 6 years ago

Providing that tests pass OK, LGTM.

Regarding the else clause, it seems that they work even with empty lists:

Python 2.7.13 (default, Jun 27 2017, 10:00:24)
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> for a in []:
...  print "loop" 
... else:
...  print "else" 
...
else
Actions
Copy link
 #17

Updated by Peter Amstutz over 6 years ago

  • Status changed from New to Resolved
  • % Done changed from 75 to 100
Actions
Copy link

Also available in: Atom PDF