From e6ad46a2a7776317acabef095a80d7fc6a817062 Mon Sep 17 00:00:00 2001 From: Josue Badillo Date: Mon, 18 Apr 2022 12:59:05 -0700 Subject: [PATCH 1/6] Feat: Adding the option to provide cpu count and memory size for the engine via the yaml project file. --- .../demo-nextflow-project/agc-project.yaml | 3 +++ .../engines/nextflow/nextflow-engine.ts | 4 ++++ packages/cdk/lib/env/context-app-parameters.ts | 11 +++++++++++ .../engines/nextflow-engine-construct.ts | 2 ++ .../pkg/cli/context/context_environment.go | 4 ++++ .../cli/internal/pkg/cli/context/manager.go | 2 ++ .../pkg/cli/context/manager_deploy_test.go | 10 +++++----- packages/cli/internal/pkg/cli/spec/context.go | 13 ++++++++++--- .../internal/pkg/cli/spec/project_schema.json | 15 +++++++++++++++ .../cli/internal/pkg/cli/spec/project_test.go | 14 ++++++++++++++ site/content/en/docs/Concepts/contexts.md | 18 ++++++++++++++++++ site/content/en/docs/Concepts/engines.md | 15 +++++++++++++++ 12 files changed, 103 insertions(+), 8 deletions(-) diff --git a/examples/demo-nextflow-project/agc-project.yaml b/examples/demo-nextflow-project/agc-project.yaml index 61bc8ab7..7b01ba3c 100644 --- a/examples/demo-nextflow-project/agc-project.yaml +++ b/examples/demo-nextflow-project/agc-project.yaml @@ -26,3 +26,6 @@ contexts: engines: - type: nextflow engine: nextflow + resourceRequirements: + vcpus: 1 + memoryLimit: 2048 diff --git a/packages/cdk/lib/constructs/engines/nextflow/nextflow-engine.ts b/packages/cdk/lib/constructs/engines/nextflow/nextflow-engine.ts index a565946d..9709c280 100644 --- a/packages/cdk/lib/constructs/engines/nextflow/nextflow-engine.ts +++ b/packages/cdk/lib/constructs/engines/nextflow/nextflow-engine.ts @@ -8,6 +8,8 @@ import { Engine, EngineProps } from "../engine"; export interface NextflowEngineProps extends EngineProps { readonly jobQueueArn: string; readonly taskRole: IRole; + readonly vcpus?: number; + readonly engineMemoryMiB?: number; } const NEXTFLOW_IMAGE_DESIGNATION = "nextflow"; @@ -21,6 +23,8 @@ export class NextflowEngine extends Engine { this.headJobDefinition = new EngineJobDefinition(this, "NexflowHeadJobDef", { logGroup: this.logGroup, container: { + vcpus: props.vcpus, + memoryLimitMiB: props.engineMemoryMiB, jobRole: props.taskRole, image: createEcrImage(this, NEXTFLOW_IMAGE_DESIGNATION), command: [], diff --git a/packages/cdk/lib/env/context-app-parameters.ts b/packages/cdk/lib/env/context-app-parameters.ts index 4f550a81..cb966cf9 100644 --- a/packages/cdk/lib/env/context-app-parameters.ts +++ b/packages/cdk/lib/env/context-app-parameters.ts @@ -87,6 +87,14 @@ export class ContextAppParameters { * The types of EC2 instances that may be launched in the compute environment. */ public readonly instanceTypes?: InstanceType[]; + /** + * The maximum number of Amazon EC2 vCPUs that an environment can reach for the nextflow engine. + */ + public readonly vCpus?: number; + /** + * The maximum memory limit that an environment can reach for the nextflow engine. + */ + public readonly memoryLimitMiB?: number; /** * If true, put EC2 instances into public subnets instead of private subnets. * This allows you to obtain significantly lower ongoing costs if used in conjunction with the usePublicSubnets option @@ -133,6 +141,9 @@ export class ContextAppParameters { this.requestSpotInstances = getEnvBoolOrDefault(node, "REQUEST_SPOT_INSTANCES", false)!; this.instanceTypes = instanceTypeStrings ? instanceTypeStrings.map((instanceType) => new InstanceType(instanceType.trim())) : undefined; + this.vCpus = getEnvNumber(node, "V_CPUS"); + this.memoryLimitMiB = getEnvNumber(node, "MEMORY_LIMIT_MIB"); + this.usePublicSubnets = getEnvBoolOrDefault(node, "PUBLIC_SUBNETS", false); this.agcVersion = getEnvString(node, "AGC_VERSION"); diff --git a/packages/cdk/lib/stacks/engines/nextflow-engine-construct.ts b/packages/cdk/lib/stacks/engines/nextflow-engine-construct.ts index b4227d73..a2fe9e1a 100644 --- a/packages/cdk/lib/stacks/engines/nextflow-engine-construct.ts +++ b/packages/cdk/lib/stacks/engines/nextflow-engine-construct.ts @@ -41,6 +41,8 @@ export class NextflowEngineConstruct extends EngineConstruct { }); this.nextflowEngine = new NextflowEngine(this, "NextflowEngine", { + vcpus: params.vCpus, + engineMemoryMiB: params.memoryLimitMiB, vpc: props.vpc, jobQueueArn: props.jobQueue.jobQueueArn, rootDirS3Uri: params.getEngineBucketPath(), diff --git a/packages/cli/internal/pkg/cli/context/context_environment.go b/packages/cli/internal/pkg/cli/context/context_environment.go index 59a23cf3..a4fdff8e 100644 --- a/packages/cli/internal/pkg/cli/context/context_environment.go +++ b/packages/cli/internal/pkg/cli/context/context_environment.go @@ -36,6 +36,8 @@ type contextEnvironment struct { ResourceType string MaxVCpus int + VCpus int + MemoryLimitMiB int RequestSpotInstances bool UsePublicSubnets bool } @@ -68,5 +70,7 @@ func (input contextEnvironment) ToEnvironmentList() []string { "MAX_V_CPUS": strconv.Itoa(input.MaxVCpus), "REQUEST_SPOT_INSTANCES": strconv.FormatBool(input.RequestSpotInstances), "PUBLIC_SUBNETS": strconv.FormatBool(input.UsePublicSubnets), + "V_CPUS": strconv.Itoa(input.VCpus), + "MEMORY_LIMIT_MIB": strconv.Itoa(input.MemoryLimitMiB), }) } diff --git a/packages/cli/internal/pkg/cli/context/manager.go b/packages/cli/internal/pkg/cli/context/manager.go index 1d9b903c..c3bc0f11 100644 --- a/packages/cli/internal/pkg/cli/context/manager.go +++ b/packages/cli/internal/pkg/cli/context/manager.go @@ -226,6 +226,8 @@ func (m *Manager) setContextEnv(contextName string) { ReadWriteBucketArns: strings.Join(m.readWriteBuckets, listDelimiter), InstanceTypes: strings.Join(m.contextSpec.InstanceTypes, listDelimiter), MaxVCpus: m.contextSpec.MaxVCpus, + VCpus: context.Engines[0].ResourceRequirements.VCpus, + MemoryLimitMiB: context.Engines[0].ResourceRequirements.MemoryLimitMiB, RequestSpotInstances: m.contextSpec.RequestSpotInstances, UsePublicSubnets: m.contextSpec.UsePublicSubnets, // TODO: we default to a single engine in a context for now diff --git a/packages/cli/internal/pkg/cli/context/manager_deploy_test.go b/packages/cli/internal/pkg/cli/context/manager_deploy_test.go index e9330092..56a3eb8c 100644 --- a/packages/cli/internal/pkg/cli/context/manager_deploy_test.go +++ b/packages/cli/internal/pkg/cli/context/manager_deploy_test.go @@ -46,7 +46,7 @@ func TestManager_Deploy(t *testing.T) { mockClients.ssmMock.EXPECT().GetCustomTags().Return(testTags) mockClients.ecrClientMock.EXPECT().VerifyImageExists(environment.CommonImages["NEXTFLOW"]).Return(nil) clearContext := mockClients.cdkMock.EXPECT().ClearContext(filepath.Join(testHomeDir, ".agc/cdk/apps/context")).Return(nil) - mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(43), testContextName3).After(clearContext).Return(mockClients.progressStream1, nil) + mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(45), testContextName3).After(clearContext).Return(mockClients.progressStream1, nil) displayProgressBar = mockClients.cdkMock.DisplayProgressBar mockClients.cdkMock.EXPECT().DisplayProgressBar(fmt.Sprintf("Deploying resources for context(s) %s", []string{testContextName3}), []cdk.ProgressStream{mockClients.progressStream1}).Return([]cdk.Result{{Outputs: []string{"some message"}, ExecutionName: testContextName3}}) return mockClients @@ -69,7 +69,7 @@ func TestManager_Deploy(t *testing.T) { mockClients.ssmMock.EXPECT().GetCustomTags().Return("") mockClients.ecrClientMock.EXPECT().VerifyImageExists(environment.CommonImages["NEXTFLOW"]).Return(nil) clearContext := mockClients.cdkMock.EXPECT().ClearContext(filepath.Join(testHomeDir, ".agc/cdk/apps/context")).Return(nil) - mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(43), testContextName3).After(clearContext).Return(mockClients.progressStream1, nil) + mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(45), testContextName3).After(clearContext).Return(mockClients.progressStream1, nil) displayProgressBar = mockClients.cdkMock.DisplayProgressBar mockClients.cdkMock.EXPECT().DisplayProgressBar(fmt.Sprintf("Deploying resources for context(s) %s", []string{testContextName3}), []cdk.ProgressStream{mockClients.progressStream1}).Return([]cdk.Result{{Outputs: []string{"some message"}, ExecutionName: testContextName3}}) return mockClients @@ -94,8 +94,8 @@ func TestManager_Deploy(t *testing.T) { mockClients.ecrClientMock.EXPECT().VerifyImageExists(environment.CommonImages["CROMWELL"]).Times(2).Return(nil) clearContext := mockClients.cdkMock.EXPECT().ClearContext(filepath.Join(testHomeDir, ".agc/cdk/apps/context")).Return(nil) clearContext2 := mockClients.cdkMock.EXPECT().ClearContext(filepath.Join(testHomeDir, ".agc/cdk/apps/context")).Return(nil) - mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(43), testContextName1).After(clearContext).Return(mockClients.progressStream1, nil) - mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(43), testContextName2).After(clearContext2).Return(mockClients.progressStream2, nil) + mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(45), testContextName1).After(clearContext).Return(mockClients.progressStream1, nil) + mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(45), testContextName2).After(clearContext2).Return(mockClients.progressStream2, nil) displayProgressBar = mockClients.cdkMock.DisplayProgressBar expectedCdkResult := []cdk.Result{{Outputs: []string{"some message"}, ExecutionName: testContextName1}, {Outputs: []string{"some other message"}, ExecutionName: testContextName2}} mockClients.cdkMock.EXPECT().DisplayProgressBar(fmt.Sprintf("Deploying resources for context(s) %s", []string{testContextName1, testContextName2}), []cdk.ProgressStream{mockClients.progressStream1, mockClients.progressStream2}).Return(expectedCdkResult) @@ -235,7 +235,7 @@ func TestManager_Deploy(t *testing.T) { mockClients.ssmMock.EXPECT().GetCommonParameter("installed-artifacts/s3-root-url").Return(testArtifactBucket, nil) mockClients.ssmMock.EXPECT().GetCustomTags().Return(testTags) mockClients.cdkMock.EXPECT().ClearContext(filepath.Join(testHomeDir, ".agc/cdk/apps/context")).Return(nil) - mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(43), testContextName1).Return(nil, fmt.Errorf("some context error")) + mockClients.cdkMock.EXPECT().DeployApp(filepath.Join(testHomeDir, ".agc/cdk/apps/context"), gomock.Len(45), testContextName1).Return(nil, fmt.Errorf("some context error")) mockClients.ecrClientMock.EXPECT().VerifyImageExists(environment.CommonImages["CROMWELL"]).Return(nil) return mockClients }, diff --git a/packages/cli/internal/pkg/cli/spec/context.go b/packages/cli/internal/pkg/cli/spec/context.go index 38fcbb02..44158997 100644 --- a/packages/cli/internal/pkg/cli/spec/context.go +++ b/packages/cli/internal/pkg/cli/spec/context.go @@ -10,10 +10,17 @@ type Filesystem struct { Configuration FSConfig `yaml:"configuration,omitempty"` } type Engine struct { - Type string `yaml:"type"` - Engine string `yaml:"engine"` - Filesystem Filesystem `yaml:"filesystem,omitempty"` + Type string `yaml:"type"` + Engine string `yaml:"engine"` + ResourceRequirements ResourceRequirement `yaml:"resourceRequirements,omitempty"` + Filesystem Filesystem `yaml:"filesystem,omitempty"` } + +type ResourceRequirement struct { + VCpus int `yaml:"vcpus,omitempty"` + MemoryLimitMiB int `yaml:"memoryLimit,omitempty"` +} + type Context struct { InstanceTypes []string `yaml:"instanceTypes,omitempty"` RequestSpotInstances bool `yaml:"requestSpotInstances,omitempty"` diff --git a/packages/cli/internal/pkg/cli/spec/project_schema.json b/packages/cli/internal/pkg/cli/spec/project_schema.json index 995f8b1c..129ad485 100644 --- a/packages/cli/internal/pkg/cli/spec/project_schema.json +++ b/packages/cli/internal/pkg/cli/spec/project_schema.json @@ -106,6 +106,21 @@ "type": "string", "minLength": 1 }, + "resourceRequirements": { + "type": "object", + "items": { + "properties": { + "vcpus":{ + "type":"integer", + "minimum": 1 + }, + "memoryLimit":{ + "type":"integer", + "minimum": 1 + } + } + } + }, "filesystem": { "type": "object", "items": { diff --git a/packages/cli/internal/pkg/cli/spec/project_test.go b/packages/cli/internal/pkg/cli/spec/project_test.go index ada31392..484c15b1 100644 --- a/packages/cli/internal/pkg/cli/spec/project_test.go +++ b/packages/cli/internal/pkg/cli/spec/project_test.go @@ -110,6 +110,10 @@ schemaVersion: 0 { Type: "nextflow", Engine: "nextflow", + ResourceRequirements: ResourceRequirement{ + VCpus: 2, + MemoryLimitMiB: 4048, + }, Filesystem: Filesystem{ FSType: "S3", }, @@ -122,6 +126,10 @@ schemaVersion: 0 { Type: "nextflow", Engine: "nextflow", + ResourceRequirements: ResourceRequirement{ + VCpus: 2, + MemoryLimitMiB: 4048, + }, }, }, }, @@ -161,6 +169,9 @@ contexts: engines: - type: nextflow engine: nextflow + resourceRequirements: + vcpus: 2 + memoryLimit: 4048 filesystem: fsType: S3 ctx3: @@ -168,6 +179,9 @@ contexts: engines: - type: nextflow engine: nextflow + resourceRequirements: + vcpus: 2 + memoryLimit: 4048 `, }, } diff --git a/site/content/en/docs/Concepts/contexts.md b/site/content/en/docs/Concepts/contexts.md index 27aee2d0..4739ed0c 100644 --- a/site/content/en/docs/Concepts/contexts.md +++ b/site/content/en/docs/Concepts/contexts.md @@ -105,7 +105,25 @@ contexts: - type: nextflow engine: nextflow ``` +### Engine vCpus and Memory +*default minimums:* +*cpus:* 1 +*memory:* 2048 +You may optionally specify the minimum number of vCpus and memory used in a context job. + +*note:* if these are not provided the defaults would be used. +```yaml +contexts: + spotContext: + requestSpotInstances: true + engines: + - type: nextflow + engine: nextflow + resourceRequirements: + vcpus: 3 + memoryLimit: 6144 +``` ### Public Subnets In the interest of saving money, in particular if you intend to have the AGC stack deployed for a long period, you may choose to deploy in "public subnet" mode. diff --git a/site/content/en/docs/Concepts/engines.md b/site/content/en/docs/Concepts/engines.md index 1726722c..6031195e 100644 --- a/site/content/en/docs/Concepts/engines.md +++ b/site/content/en/docs/Concepts/engines.md @@ -51,6 +51,21 @@ contexts: engine: cromwell ``` +The following snippet +defines a Nextflow DSL engine of type `Nextflow` as part of the context named `spotContext` with a minimum requirement count of Cpus and Memory, when these are not provided the defaults would be used. (cpus:1, memory: 2048) + +```yaml +contexts: + spotContext: + requestSpotInstances: true + engines: + - type: nextflow + engine: nextflow + resourceRequirements: + vcpus: 1 + memoryLimit: 2048 +``` + ## Commands There are no commands specific to engines. Engines are [deployed]( {{< relref "contexts#deploy" >}} ) along with contexts by the [`context` commands]( {{< relref "contexts#context-commands" >}} ) and workflows From df06b26295a0985e94389ddfb8fd69fd98c8861e Mon Sep 17 00:00:00 2001 From: Josue Badillo Date: Mon, 18 Apr 2022 12:59:05 -0700 Subject: [PATCH 2/6] Feat: Adding the option to provide cpu count and memory size for the engine via the yaml project file. --- examples/demo-nextflow-project/agc-project.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/demo-nextflow-project/agc-project.yaml b/examples/demo-nextflow-project/agc-project.yaml index 7b01ba3c..a6d4d9d2 100644 --- a/examples/demo-nextflow-project/agc-project.yaml +++ b/examples/demo-nextflow-project/agc-project.yaml @@ -27,5 +27,5 @@ contexts: - type: nextflow engine: nextflow resourceRequirements: - vcpus: 1 + vcpus: 2 memoryLimit: 2048 From eeda2422e507cc307bebd3d3cabac9005e8da65e Mon Sep 17 00:00:00 2001 From: Josue Badillo Date: Mon, 18 Apr 2022 12:59:05 -0700 Subject: [PATCH 3/6] Feat: Adding the option to provide cpu count and memory size for the engine via the yaml project file. --- examples/demo-nextflow-project/agc-project.yaml | 2 +- examples/demo-snakemake-project/agc-project.yaml | 3 +++ examples/demo-wdl-project/agc-project.yaml | 3 +++ .../cdk/lib/constructs/engines/engine-job-definition.ts | 4 ++-- .../cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts | 6 ++++-- .../lib/constructs/engines/snakemake/snakemake-engine.ts | 6 ++++-- packages/cdk/lib/stacks/engines/miniwdl-engine-construct.ts | 2 ++ .../cdk/lib/stacks/engines/snakemake-engine-construct.ts | 2 ++ 8 files changed, 21 insertions(+), 7 deletions(-) diff --git a/examples/demo-nextflow-project/agc-project.yaml b/examples/demo-nextflow-project/agc-project.yaml index a6d4d9d2..e33cca39 100644 --- a/examples/demo-nextflow-project/agc-project.yaml +++ b/examples/demo-nextflow-project/agc-project.yaml @@ -28,4 +28,4 @@ contexts: engine: nextflow resourceRequirements: vcpus: 2 - memoryLimit: 2048 + memoryLimit: 4096 diff --git a/examples/demo-snakemake-project/agc-project.yaml b/examples/demo-snakemake-project/agc-project.yaml index 03292672..26c75ae2 100644 --- a/examples/demo-snakemake-project/agc-project.yaml +++ b/examples/demo-snakemake-project/agc-project.yaml @@ -16,6 +16,9 @@ contexts: engines: - type: snakemake engine: snakemake + resourceRequirements: + vcpus: 2 + memoryLimit: 4096 spotContext: requestSpotInstances: true engines: diff --git a/examples/demo-wdl-project/agc-project.yaml b/examples/demo-wdl-project/agc-project.yaml index 9888e8d0..3c1ec005 100644 --- a/examples/demo-wdl-project/agc-project.yaml +++ b/examples/demo-wdl-project/agc-project.yaml @@ -37,6 +37,9 @@ contexts: engines: - type: wdl engine: miniwdl + resourceRequirements: + vcpus: 2 + memoryLimit: 4096 spotCtx: requestSpotInstances: true diff --git a/packages/cdk/lib/constructs/engines/engine-job-definition.ts b/packages/cdk/lib/constructs/engines/engine-job-definition.ts index 64af9834..c38bfc88 100644 --- a/packages/cdk/lib/constructs/engines/engine-job-definition.ts +++ b/packages/cdk/lib/constructs/engines/engine-job-definition.ts @@ -19,8 +19,8 @@ export class EngineJobDefinition extends JobDefinition { AWS_METADATA_SERVICE_NUM_ATTEMPTS: "10", ...props.container.environment, }, - memoryLimitMiB: props.container.memoryLimitMiB || 2048, - vcpus: props.container.vcpus || 1, + memoryLimitMiB: props.container.memoryLimitMiB || 4096, + vcpus: props.container.vcpus || 2, }, }); } diff --git a/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts b/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts index 8f185575..35c3166f 100644 --- a/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts +++ b/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts @@ -12,13 +12,14 @@ export interface MiniWdlEngineProps extends EngineProps { readonly engineBatch: Batch; readonly workerBatch: Batch; readonly iops?: Size; + readonly vcpus?: number; + readonly engineMemoryMiB?: number; } const MINIWDL_IMAGE_DESIGNATION = "miniwdl"; export class MiniWdlEngine extends Engine { readonly headJobDefinition: JobDefinition; - private readonly engineMemoryMiB = 4096; private readonly volumeName = "efs"; readonly fileSystem: FileSystem; @@ -41,7 +42,8 @@ export class MiniWdlEngine extends Engine { logGroup: this.logGroup, platformCapabilities: [PlatformCapabilities.FARGATE], container: { - memoryLimitMiB: this.engineMemoryMiB, + vcpus: props.vcpus, + memoryLimitMiB: props.engineMemoryMiB, jobRole: engineBatch.role, executionRole: engineBatch.role, image: createEcrImage(this, MINIWDL_IMAGE_DESIGNATION), diff --git a/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts b/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts index eb648608..564e9e15 100644 --- a/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts +++ b/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts @@ -12,6 +12,8 @@ export interface SnakemakeEngineProps extends EngineProps { readonly engineBatch: Batch; readonly workerBatch: Batch; readonly iops?: Size; + readonly vcpus?: number; + readonly engineMemoryMiB?: number; } const SNAKEMAKE_IMAGE_DESIGNATION = "snakemake"; @@ -19,7 +21,6 @@ const SNAKEMAKE_IMAGE_DESIGNATION = "snakemake"; export class SnakemakeEngine extends Engine { readonly headJobDefinition: JobDefinition; private readonly volumeName = "efs"; - private readonly engineMemoryMiB = 4096; public readonly fsap: AccessPoint; public readonly fileSystem: FileSystem; @@ -41,7 +42,8 @@ export class SnakemakeEngine extends Engine { logGroup: this.logGroup, platformCapabilities: [PlatformCapabilities.FARGATE], container: { - memoryLimitMiB: this.engineMemoryMiB, + vcpus: props.vcpus, + memoryLimitMiB: props.engineMemoryMiB, jobRole: engineBatch.role, executionRole: engineBatch.role, image: createEcrImage(this, SNAKEMAKE_IMAGE_DESIGNATION), diff --git a/packages/cdk/lib/stacks/engines/miniwdl-engine-construct.ts b/packages/cdk/lib/stacks/engines/miniwdl-engine-construct.ts index 935f5b71..f8396acb 100644 --- a/packages/cdk/lib/stacks/engines/miniwdl-engine-construct.ts +++ b/packages/cdk/lib/stacks/engines/miniwdl-engine-construct.ts @@ -52,6 +52,8 @@ export class MiniwdlEngineConstruct extends EngineConstruct { ); this.miniwdlEngine = new MiniWdlEngine(this, "MiniWdlEngine", { + vcpus: params.vCpus, + engineMemoryMiB: params.memoryLimitMiB, vpc: props.vpc, iops: props.iops, rootDirS3Uri: rootDirS3Uri, diff --git a/packages/cdk/lib/stacks/engines/snakemake-engine-construct.ts b/packages/cdk/lib/stacks/engines/snakemake-engine-construct.ts index b7a73b51..c5cb65a4 100644 --- a/packages/cdk/lib/stacks/engines/snakemake-engine-construct.ts +++ b/packages/cdk/lib/stacks/engines/snakemake-engine-construct.ts @@ -92,6 +92,8 @@ export class SnakemakeEngineConstruct extends EngineConstruct { private createSnakemakeEngine(props: EngineOptions, batchHead: Batch, batchWorkers: Batch): SnakemakeEngine { return new SnakemakeEngine(this, "SnakemakeEngine", { + vcpus: props.contextParameters.vCpus, + engineMemoryMiB: props.contextParameters.memoryLimitMiB, vpc: props.vpc, iops: props.iops, engineBatch: batchHead, From 48e327cc18b94cb00624a7e0defd797ef399cec9 Mon Sep 17 00:00:00 2001 From: Josue Badillo Date: Mon, 18 Apr 2022 12:59:05 -0700 Subject: [PATCH 4/6] Feat: Adding the option to provide cpu count and memory size for the engine via the yaml project file. --- site/content/en/docs/Concepts/contexts.md | 6 +++--- site/content/en/docs/Concepts/engines.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/site/content/en/docs/Concepts/contexts.md b/site/content/en/docs/Concepts/contexts.md index 4739ed0c..bb9f9959 100644 --- a/site/content/en/docs/Concepts/contexts.md +++ b/site/content/en/docs/Concepts/contexts.md @@ -107,8 +107,8 @@ contexts: ``` ### Engine vCpus and Memory *default minimums:* -*cpus:* 1 -*memory:* 2048 +*cpus:* 2 +*memory:* 4096 You may optionally specify the minimum number of vCpus and memory used in a context job. @@ -121,7 +121,7 @@ contexts: - type: nextflow engine: nextflow resourceRequirements: - vcpus: 3 + vcpus: 2 memoryLimit: 6144 ``` ### Public Subnets diff --git a/site/content/en/docs/Concepts/engines.md b/site/content/en/docs/Concepts/engines.md index 6031195e..340b6dd1 100644 --- a/site/content/en/docs/Concepts/engines.md +++ b/site/content/en/docs/Concepts/engines.md @@ -52,7 +52,7 @@ contexts: ``` The following snippet -defines a Nextflow DSL engine of type `Nextflow` as part of the context named `spotContext` with a minimum requirement count of Cpus and Memory, when these are not provided the defaults would be used. (cpus:1, memory: 2048) +defines a Nextflow DSL engine of type `Nextflow` as part of the context named `spotContext` with a minimum requirement count of Cpus and Memory, when these are not provided the defaults would be used. (cpus:2, memory: 4096) ```yaml contexts: @@ -62,8 +62,8 @@ contexts: - type: nextflow engine: nextflow resourceRequirements: - vcpus: 1 - memoryLimit: 2048 + vcpus: 2 + memoryLimit: 4096 ``` ## Commands From 4c9ad65b613b9692829d01461539a3c2147d2555 Mon Sep 17 00:00:00 2001 From: Josue Badillo Date: Tue, 24 May 2022 17:09:02 -0700 Subject: [PATCH 5/6] Feat: Adding the option to provide cpu count and memory size for the all engines via the yaml project file. --- .../demo-nextflow-project/agc-project.yaml | 3 --- .../demo-snakemake-project/agc-project.yaml | 3 --- examples/demo-wdl-project/agc-project.yaml | 3 --- .../engines/engine-job-definition.ts | 4 ++-- .../engines/miniwdl/miniwdl-engine.ts | 4 ++-- .../engines/snakemake/snakemake-engine.ts | 4 ++-- .../engines/cromwell-engine-construct.ts | 19 +++++++++++++------ .../stacks/engines/toil-engine-construct.ts | 19 +++++++++++++------ site/content/en/docs/Concepts/contexts.md | 18 ++++++++++++------ site/content/en/docs/Concepts/engines.md | 2 +- 10 files changed, 45 insertions(+), 34 deletions(-) diff --git a/examples/demo-nextflow-project/agc-project.yaml b/examples/demo-nextflow-project/agc-project.yaml index 69d0a3f3..95a923d6 100644 --- a/examples/demo-nextflow-project/agc-project.yaml +++ b/examples/demo-nextflow-project/agc-project.yaml @@ -31,6 +31,3 @@ contexts: engines: - type: nextflow engine: nextflow - resourceRequirements: - vcpus: 2 - memoryLimit: 4096 diff --git a/examples/demo-snakemake-project/agc-project.yaml b/examples/demo-snakemake-project/agc-project.yaml index d931da13..2683cb4e 100644 --- a/examples/demo-snakemake-project/agc-project.yaml +++ b/examples/demo-snakemake-project/agc-project.yaml @@ -16,6 +16,3 @@ contexts: engines: - type: snakemake engine: snakemake - resourceRequirements: - vcpus: 2 - memoryLimit: 4096 diff --git a/examples/demo-wdl-project/agc-project.yaml b/examples/demo-wdl-project/agc-project.yaml index 1b0bfc67..6595080c 100644 --- a/examples/demo-wdl-project/agc-project.yaml +++ b/examples/demo-wdl-project/agc-project.yaml @@ -32,9 +32,6 @@ contexts: engines: - type: wdl engine: miniwdl - resourceRequirements: - vcpus: 2 - memoryLimit: 4096 spotCtx: requestSpotInstances: true diff --git a/packages/cdk/lib/constructs/engines/engine-job-definition.ts b/packages/cdk/lib/constructs/engines/engine-job-definition.ts index c38bfc88..64af9834 100644 --- a/packages/cdk/lib/constructs/engines/engine-job-definition.ts +++ b/packages/cdk/lib/constructs/engines/engine-job-definition.ts @@ -19,8 +19,8 @@ export class EngineJobDefinition extends JobDefinition { AWS_METADATA_SERVICE_NUM_ATTEMPTS: "10", ...props.container.environment, }, - memoryLimitMiB: props.container.memoryLimitMiB || 4096, - vcpus: props.container.vcpus || 2, + memoryLimitMiB: props.container.memoryLimitMiB || 2048, + vcpus: props.container.vcpus || 1, }, }); } diff --git a/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts b/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts index d837c899..316d97d6 100644 --- a/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts +++ b/packages/cdk/lib/constructs/engines/miniwdl/miniwdl-engine.ts @@ -42,8 +42,8 @@ export class MiniWdlEngine extends Engine { logGroup: this.logGroup, platformCapabilities: [PlatformCapabilities.FARGATE], container: { - vcpus: props.vcpus, - memoryLimitMiB: props.engineMemoryMiB, + vcpus: props.vcpus || 1, + memoryLimitMiB: props.engineMemoryMiB || 4096, jobRole: engineBatch.role, executionRole: engineBatch.role, image: createEcrImage(this, MINIWDL_IMAGE_DESIGNATION), diff --git a/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts b/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts index 7c37257b..75e6290b 100644 --- a/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts +++ b/packages/cdk/lib/constructs/engines/snakemake/snakemake-engine.ts @@ -42,8 +42,8 @@ export class SnakemakeEngine extends Engine { logGroup: this.logGroup, platformCapabilities: [PlatformCapabilities.FARGATE], container: { - vcpus: props.vcpus, - memoryLimitMiB: props.engineMemoryMiB, + vcpus: props.vcpus || 1, + memoryLimitMiB: props.engineMemoryMiB || 4096, jobRole: engineBatch.role, executionRole: engineBatch.role, image: createEcrImage(this, SNAKEMAKE_IMAGE_DESIGNATION), diff --git a/packages/cdk/lib/stacks/engines/cromwell-engine-construct.ts b/packages/cdk/lib/stacks/engines/cromwell-engine-construct.ts index eed30d35..7ec67a2b 100644 --- a/packages/cdk/lib/stacks/engines/cromwell-engine-construct.ts +++ b/packages/cdk/lib/stacks/engines/cromwell-engine-construct.ts @@ -14,6 +14,7 @@ import { CromwellEngineRole } from "../../roles/cromwell-engine-role"; import { CromwellAdapterRole } from "../../roles/cromwell-adapter-role"; import { IJobQueue } from "@aws-cdk/aws-batch-alpha"; import { Construct } from "constructs"; +import {ContextAppParameters} from "../../env"; export interface CromwellEngineConstructProps extends EngineOptions { /** @@ -57,7 +58,7 @@ export class CromwellEngineConstruct extends EngineConstruct { }); // TODO: Move log group creation into service construct and make it a property - this.engine = this.getEngineServiceDefinition(props.vpc, props.subnets, engineContainer, this.engineLogGroup); + this.engine = this.getEngineServiceDefinition(props.vpc, props.subnets, engineContainer, this.engineLogGroup, params); this.adapterLogGroup = new LogGroup(this, "AdapterLogGroup"); const lambda = this.renderAdapterLambda({ @@ -89,7 +90,13 @@ export class CromwellEngineConstruct extends EngineConstruct { }; } - private getEngineServiceDefinition(vpc: IVpc, subnets: SubnetSelection, serviceContainer: ServiceContainer, logGroup: ILogGroup) { + private getEngineServiceDefinition( + vpc: IVpc, + subnets: SubnetSelection, + serviceContainer: ServiceContainer, + logGroup: ILogGroup, + params: ContextAppParameters + ) { const id = "Engine"; const fileSystem = new FileSystem(this, "EngineFileSystem", { vpc, @@ -99,8 +106,8 @@ export class CromwellEngineConstruct extends EngineConstruct { }); const definition = new FargateTaskDefinition(this, "EngineTaskDef", { taskRole: this.engineRole, - cpu: serviceContainer.cpu, - memoryLimitMiB: serviceContainer.memoryLimitMiB, + cpu: (params.vCpus ? params.vCpus * 1024 : undefined) || serviceContainer.cpu, + memoryLimitMiB: params.memoryLimitMiB || serviceContainer.memoryLimitMiB, }); const volumeName = "cromwell-executions"; @@ -112,8 +119,8 @@ export class CromwellEngineConstruct extends EngineConstruct { }); const container = definition.addContainer(serviceContainer.serviceName, { - cpu: serviceContainer.cpu, - memoryLimitMiB: serviceContainer.memoryLimitMiB, + cpu: (params.vCpus ? params.vCpus * 1024 : undefined) || serviceContainer.cpu, + memoryLimitMiB: params.memoryLimitMiB || serviceContainer.memoryLimitMiB, environment: serviceContainer.environment, containerName: serviceContainer.serviceName, image: createEcrImage(this, serviceContainer.imageConfig.designation), diff --git a/packages/cdk/lib/stacks/engines/toil-engine-construct.ts b/packages/cdk/lib/stacks/engines/toil-engine-construct.ts index 0ae6b91d..2c790c35 100644 --- a/packages/cdk/lib/stacks/engines/toil-engine-construct.ts +++ b/packages/cdk/lib/stacks/engines/toil-engine-construct.ts @@ -12,6 +12,7 @@ import { ToilJobRole } from "../../roles/toil-job-role"; import { ToilEngineRole } from "../../roles/toil-engine-role"; import { IJobQueue } from "@aws-cdk/aws-batch-alpha"; import { Construct } from "constructs"; +import { ContextAppParameters } from "../../env"; export interface ToilEngineConstructProps extends EngineOptions { /** @@ -55,7 +56,7 @@ export class ToilEngineConstruct extends EngineConstruct { TOIL_AWS_BATCH_JOB_ROLE_ARN: this.jobRole.roleArn, }); - this.engine = this.getEngineServiceDefinition(props.vpc, props.subnets, engineContainer, this.engineLogGroup); + this.engine = this.getEngineServiceDefinition(props.vpc, props.subnets, engineContainer, this.engineLogGroup, params); // We don't use an adapter, so put the access-controlling proxy right in // front of the engine load balancer. @@ -74,17 +75,23 @@ export class ToilEngineConstruct extends EngineConstruct { }; } - private getEngineServiceDefinition(vpc: IVpc, subnets: SubnetSelection, serviceContainer: ServiceContainer, logGroup: ILogGroup) { + private getEngineServiceDefinition( + vpc: IVpc, + subnets: SubnetSelection, + serviceContainer: ServiceContainer, + logGroup: ILogGroup, + params: ContextAppParameters + ) { const id = "Engine"; const definition = new FargateTaskDefinition(this, "EngineTaskDef", { taskRole: this.engineRole, - cpu: serviceContainer.cpu, - memoryLimitMiB: serviceContainer.memoryLimitMiB, + cpu: (params.vCpus ? params.vCpus * 1024 : undefined) || serviceContainer.cpu, + memoryLimitMiB: params.memoryLimitMiB || serviceContainer.memoryLimitMiB, }); definition.addContainer(serviceContainer.serviceName, { - cpu: serviceContainer.cpu, - memoryLimitMiB: serviceContainer.memoryLimitMiB, + cpu: (params.vCpus ? params.vCpus * 1024 : undefined) || serviceContainer.cpu, + memoryLimitMiB: params.memoryLimitMiB || serviceContainer.memoryLimitMiB, environment: serviceContainer.environment, containerName: serviceContainer.serviceName, image: createEcrImage(this, serviceContainer.imageConfig.designation), diff --git a/site/content/en/docs/Concepts/contexts.md b/site/content/en/docs/Concepts/contexts.md index bb9f9959..6ee1aff2 100644 --- a/site/content/en/docs/Concepts/contexts.md +++ b/site/content/en/docs/Concepts/contexts.md @@ -105,14 +105,20 @@ contexts: - type: nextflow engine: nextflow ``` -### Engine vCpus and Memory -*default minimums:* -*cpus:* 2 -*memory:* 4096 +### Engine vCpus and Memory default minimums: -You may optionally specify the minimum number of vCpus and memory used in a context job. +| Engine | vCpus | Memory value (MiB) | +|-----------|-------|--------------------| +| Cromwell | 2 | 16384 | +| Nextflow | 1 | 2048 | +| miniwdl | 2 | 4096 | +| Snakemake | 2 | 4096 | +| Toil | 2 | 16384 | -*note:* if these are not provided the defaults would be used. + +You may optionally specify the number of vCpus and memory used in a context job, vCpu count for engines Cromwell or Toil would be converted to cpu units, 1024 per each vCPU. + +*note:* if these are not provided the defaults on the table above would be used. ```yaml contexts: spotContext: diff --git a/site/content/en/docs/Concepts/engines.md b/site/content/en/docs/Concepts/engines.md index 4e20f68f..da7e93fc 100644 --- a/site/content/en/docs/Concepts/engines.md +++ b/site/content/en/docs/Concepts/engines.md @@ -53,7 +53,7 @@ contexts: ``` The following snippet -defines a Nextflow DSL engine of type `Nextflow` as part of the context named `spotContext` with a minimum requirement count of Cpus and Memory, when these are not provided the defaults would be used. (cpus:2, memory: 4096) +defines a Nextflow DSL engine of type `Nextflow` as part of the context named `spotContext` with a minimum requirement count of Cpus and Memory, when these are not provided the defaults would be used. ```yaml contexts: From 7d28f6b2a3f5ba207f199d89114c3d8822f53839 Mon Sep 17 00:00:00 2001 From: Josue Badillo <98841162+josuebc@users.noreply.github.com> Date: Wed, 25 May 2022 09:26:21 -0700 Subject: [PATCH 6/6] Update site/content/en/docs/Concepts/contexts.md Co-authored-by: Mark Schreiber --- site/content/en/docs/Concepts/contexts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/docs/Concepts/contexts.md b/site/content/en/docs/Concepts/contexts.md index 6ee1aff2..d274edea 100644 --- a/site/content/en/docs/Concepts/contexts.md +++ b/site/content/en/docs/Concepts/contexts.md @@ -116,7 +116,7 @@ contexts: | Toil | 2 | 16384 | -You may optionally specify the number of vCpus and memory used in a context job, vCpu count for engines Cromwell or Toil would be converted to cpu units, 1024 per each vCPU. +You may optionally specify the number of vCpus and memory used in a context definition, vCpu count for server engines such as Cromwell or Toil would be converted to cpu units, 1024 per each vCPU. *note:* if these are not provided the defaults on the table above would be used. ```yaml