Skip to content

CDAP-21264 : Respect custom CPU and memory multipliers for Task Workers #16196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sahusanket wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

CDAP-21264 : Respect custom CPU and memory multipliers for Task Workers #16196

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96bf8cb
CDAP-21264 : Respect custom CPU and memory multipliers for Task Workers
sahusanket Aug 10, 2026
b2ba64a
Respect custom CPU and memory multipliers for Artifact localizer
sahusanket Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/TaskWorkerServiceLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ public void run() {
Map<String, String> configMap = new HashMap<>();
configMap.put(ProgramOptionConstants.RUNTIME_NAMESPACE,
NamespaceId.SYSTEM.getNamespace());
if (cConf.get(Constants.TaskWorker.CONTAINER_CPU_MULTIPLIER) != null) {
configMap.put(Constants.Kube.CPU_MULTIPLIER,
cConf.get(Constants.TaskWorker.CONTAINER_CPU_MULTIPLIER));
}
if (cConf.get(Constants.TaskWorker.CONTAINER_MEMORY_MULTIPLIER) != null) {
configMap.put(Constants.Kube.MEMORY_MULTIPLIER,
cConf.get(Constants.TaskWorker.CONTAINER_MEMORY_MULTIPLIER));
}
Comment on lines +206 to +213

@gemini-code-assist gemini-code-assist Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To avoid redundant lookups in cConf (which can be expensive as it resolves configuration variables), store the retrieved multiplier values in local variables instead of calling cConf.get() multiple times for the same key.

Suggested change
if (cConf.get(Constants.TaskWorker.CONTAINER_CPU_MULTIPLIER) != null) {
configMap.put(Constants.Kube.CPU_MULTIPLIER,
cConf.get(Constants.TaskWorker.CONTAINER_CPU_MULTIPLIER));
}
if (cConf.get(Constants.TaskWorker.CONTAINER_MEMORY_MULTIPLIER) != null) {
configMap.put(Constants.Kube.MEMORY_MULTIPLIER,
cConf.get(Constants.TaskWorker.CONTAINER_MEMORY_MULTIPLIER));
}
String cpuMultiplier = cConf.get(Constants.TaskWorker.CONTAINER_CPU_MULTIPLIER);
if (cpuMultiplier != null) {
configMap.put(Constants.Kube.CPU_MULTIPLIER, cpuMultiplier);
}
String memoryMultiplier = cConf.get(Constants.TaskWorker.CONTAINER_MEMORY_MULTIPLIER);
if (memoryMultiplier != null) {
configMap.put(Constants.Kube.MEMORY_MULTIPLIER, memoryMultiplier);
}

twillPreparer.withConfiguration(Collections.unmodifiableMap(configMap));

if (Feature.NAMESPACED_SERVICE_ACCOUNTS.isEnabled(featureFlagsProvider)) {
Expand Down
8 changes: 8 additions & 0 deletions cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,14 @@
*/
public static final String PROGRAM_SUBMISSION_MASTER_ENV_ENABLED = "program.submission.master.environment.enabled";
}
/**
* Kubernetes constants.
* These same as defined in KubeTwillPreparer
*/
Comment on lines +493 to +496

@gemini-code-assist gemini-code-assist Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Fix the grammatical typo in the Javadoc comment and update it to reflect that these are the centralized constants used by KubeTwillPreparer.

Suggested change
/**
* Kubernetes constants.
* These same as defined in KubeTwillPreparer
*/
/**
* Kubernetes constants.
* These are used by KubeTwillPreparer.
*/

public static final class Kube {

Check warning on line 497 in cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyLineSeparatorCheck 

'CLASS_DEF' should be separated from previous line.

Check warning on line 497 in cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyLineSeparatorCheck 

'CLASS_DEF' should be separated from previous line.

Check warning on line 497 in cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a private constructor to hide the implicit public one. 


See more on https://sonarcloud.io/project/issues?id=cdapio_cdap&issues=AZ_sMXoSaUblf6BY5uca&open=AZ_sMXoSaUblf6BY5uca&pullRequest=16196
public static final String CPU_MULTIPLIER = "master.environment.k8s.container.cpu.multiplier";
public static final String MEMORY_MULTIPLIER = "master.environment.k8s.container.memory.multiplier";
}

/**
* Task worker.
Expand Down
13 changes: 11 additions & 2 deletions cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@
private StringBuilder globalJvmOptions;
private final V1EmptyDirVolumeSource workDirVolumeSource;
private boolean shouldLocalizeConfigurationAsConfigmap;
private String systemCpuMultiplier;
private String systemMemoryMultiplier;

KubeTwillPreparer(MasterEnvironmentContext masterEnvContext, ApiClient apiClient,
String kubeNamespace,
Expand Down Expand Up @@ -436,6 +438,12 @@
if (config.containsKey(MasterOptionConstants.RUNTIME_NAMESPACE)) {
cdapRuntimeNamespace = config.get(MasterOptionConstants.RUNTIME_NAMESPACE);
}
if (config.containsKey(CPU_MULTIPLIER)) {
systemCpuMultiplier = config.get(CPU_MULTIPLIER);
}
if (config.containsKey(MEMORY_MULTIPLIER)) {
systemMemoryMultiplier = config.get(MEMORY_MULTIPLIER);
}
Comment on lines +441 to +446

@gemini-code-assist gemini-code-assist Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of using the locally defined CPU_MULTIPLIER and MEMORY_MULTIPLIER constants (which duplicates the configuration keys), use the newly introduced centralized constants Constants.Kube.CPU_MULTIPLIER and Constants.Kube.MEMORY_MULTIPLIER from cdap-common to ensure consistency and maintainability.

Suggested change
if (config.containsKey(CPU_MULTIPLIER)) {
systemCpuMultiplier = config.get(CPU_MULTIPLIER);
}
if (config.containsKey(MEMORY_MULTIPLIER)) {
systemMemoryMultiplier = config.get(MEMORY_MULTIPLIER);
}
if (config.containsKey(Constants.Kube.CPU_MULTIPLIER)) {
systemCpuMultiplier = config.get(Constants.Kube.CPU_MULTIPLIER);
}
if (config.containsKey(Constants.Kube.MEMORY_MULTIPLIER)) {
systemMemoryMultiplier = config.get(Constants.Kube.MEMORY_MULTIPLIER);
}

for (String runnableName : runnables) {
withEnv(runnableName, config);
}
Expand Down Expand Up @@ -640,7 +648,7 @@
}
}

@Override

Check warning on line 651 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.OverloadMethodsDeclarationOrderCheck 

All overloaded methods should be placed next to each other. Placing non-overloaded methods in between overloaded methods with the same type is a violation. Previous overloaded method located at line '636'.

Check warning on line 651 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.OverloadMethodsDeclarationOrderCheck 

All overloaded methods should be placed next to each other. Placing non-overloaded methods in between overloaded methods with the same type is a violation. Previous overloaded method located at line '636'.
public TwillController start(long timeout, TimeUnit timeoutUnit) {
validateSpecification();
try {
Expand Down Expand Up @@ -1006,10 +1014,10 @@
if (memory == null) {
throw new IllegalArgumentException("No memory settings in the given resource requirements");
}
int memoryMB = (int) (memory.getNumber().longValue() >> 20);

Check warning on line 1017 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'memoryMB' must contain no more than '1' consecutive capital letters.

Check warning on line 1017 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'memoryMB' must contain no more than '1' consecutive capital letters.

Map<String, String> cConf = masterEnvContext.getConfigurations();
int reservedMemoryMB = Integer.parseInt(cConf.get(Configs.Keys.JAVA_RESERVED_MEMORY_MB));

Check warning on line 1020 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'reservedMemoryMB' must contain no more than '1' consecutive capital letters.

Check warning on line 1020 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'reservedMemoryMB' must contain no more than '1' consecutive capital letters.
double minHeapRatio = Double.parseDouble(cConf.get(Configs.Keys.HEAP_RESERVED_MIN_RATIO));
return org.apache.twill.internal.utils.Resources.computeMaxHeapSize(memoryMB, reservedMemoryMB,
minHeapRatio);
Expand Down Expand Up @@ -1336,7 +1344,7 @@
.toArray(String[]::new)));

for (String name : this.dependentRunnableNames) {
RuntimeSpecification spec = runtimeSpecs.get(name);

Check warning on line 1347 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'spec' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).

Check warning on line 1347 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'spec' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).
// Add all environments for the runnable
environs.putAll(environments.get(name));
// Add JVM options to environment.
Expand Down Expand Up @@ -1391,7 +1399,7 @@
// Set the process memory is through the JAVA_HEAPMAX variable.
environs.put("JAVA_HEAPMAX",
String.format("-Xmx%dm", computeMaxHeapSize(resourceRequirements)));
List<V1EnvVar> containerEnvironments = environs.entrySet().stream()

Check warning on line 1402 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'containerEnvironments' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).

Check warning on line 1402 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'containerEnvironments' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).
.map(e -> new V1EnvVar().name(e.getKey()).value(e.getValue()))
.collect(Collectors.toList());

Expand All @@ -1416,7 +1424,7 @@
}
}

if (containerProbes.containsKey(name)){

Check warning on line 1427 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAroundCheck 

WhitespaceAround: '{' is not preceded with whitespace.

Check warning on line 1427 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAroundCheck 

WhitespaceAround: '{' is not preceded with whitespace.
containerProbes.get(name).forEach((probeName, probeObj) -> {
switch (probeName) {
case LIVENESS:
Expand Down Expand Up @@ -1454,8 +1462,9 @@
@VisibleForTesting
V1ResourceRequirements createResourceRequirements(ResourceSpecification resourceSpec) {
Map<String, String> cConf = masterEnvContext.getConfigurations();
float cpuMultiplier = Float.parseFloat(cConf.getOrDefault(CPU_MULTIPLIER, DEFAULT_MULTIPLIER));
float memoryMultiplier = Float.parseFloat(
float cpuMultiplier = Float.parseFloat(systemCpuMultiplier != null ? systemCpuMultiplier :
cConf.getOrDefault(CPU_MULTIPLIER, DEFAULT_MULTIPLIER));
float memoryMultiplier = Float.parseFloat(systemMemoryMultiplier != null ? systemMemoryMultiplier :
cConf.getOrDefault(MEMORY_MULTIPLIER, DEFAULT_MULTIPLIER));

@gemini-code-assist gemini-code-assist Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use the centralized constants Constants.Kube.CPU_MULTIPLIER and Constants.Kube.MEMORY_MULTIPLIER here as well to avoid relying on duplicate local constants.

Suggested change
float cpuMultiplier = Float.parseFloat(systemCpuMultiplier != null ? systemCpuMultiplier :
cConf.getOrDefault(CPU_MULTIPLIER, DEFAULT_MULTIPLIER));
float memoryMultiplier = Float.parseFloat(systemMemoryMultiplier != null ? systemMemoryMultiplier :
cConf.getOrDefault(MEMORY_MULTIPLIER, DEFAULT_MULTIPLIER));
float cpuMultiplier = Float.parseFloat(systemCpuMultiplier != null ? systemCpuMultiplier :
cConf.getOrDefault(Constants.Kube.CPU_MULTIPLIER, DEFAULT_MULTIPLIER));
float memoryMultiplier = Float.parseFloat(systemMemoryMultiplier != null ? systemMemoryMultiplier :
cConf.getOrDefault(Constants.Kube.MEMORY_MULTIPLIER, DEFAULT_MULTIPLIER));


V1ResourceRequirementsBuilder requirementsBuilder = new V1ResourceRequirementsBuilder();
Expand Down Expand Up @@ -1626,9 +1635,9 @@
* Get {@link Map} of properties prefixed with the string provided as an input.
* Property names in the mapping are trimmed to remove the prefix.
*
* @param originalMap

Check warning on line 1638 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.

Check warning on line 1638 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.
* @param prefix

Check warning on line 1639 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.

Check warning on line 1639 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.
* @return

Check warning on line 1640 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.

Check warning on line 1640 in cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.NonEmptyAtclauseDescriptionCheck 

At-clause should have a non-empty description.
*/
public static Map<String, String> filterAndRemoveKeyPrefix(Map<String, String> originalMap,
String prefix) {
Expand Down
8 changes: 2 additions & 6 deletions cdap-master/src/main/java/io/cdap/cdap/master/environment/k8s/PreviewServiceMain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,7 @@
*/
public class PreviewServiceMain extends AbstractServiceMain<EnvironmentOptions> {

// Following constants are same as defined in AbstractKubeTwillPreparer
private static final String KUBE_CPU_MULTIPLIER = "master.environment.k8s.container.cpu.multiplier";
private static final String KUBE_MEMORY_MULTIPLIER = "master.environment.k8s.container.memory.multiplier";

/**

Check warning on line 68 in cdap-master/src/main/java/io/cdap/cdap/master/environment/k8s/PreviewServiceMain.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.SummaryJavadocCheck 

First sentence of Javadoc is missing an ending period.

Check warning on line 68 in cdap-master/src/main/java/io/cdap/cdap/master/environment/k8s/PreviewServiceMain.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.javadoc.SummaryJavadocCheck 

First sentence of Javadoc is missing an ending period.
* Main entry point
*/
public static void main(String[] args) throws Exception {
Expand All @@ -79,8 +75,8 @@
@Override
protected CConfiguration updateCConf(CConfiguration cConf) {
Map<String, String> keyMap = ImmutableMap.of(
Constants.Preview.CONTAINER_CPU_MULTIPLIER, KUBE_CPU_MULTIPLIER,
Constants.Preview.CONTAINER_MEMORY_MULTIPLIER, KUBE_MEMORY_MULTIPLIER,
Constants.Preview.CONTAINER_CPU_MULTIPLIER, Constants.Kube.CPU_MULTIPLIER,
Constants.Preview.CONTAINER_MEMORY_MULTIPLIER, Constants.Kube.MEMORY_MULTIPLIER,
Constants.Preview.CONTAINER_HEAP_RESERVED_RATIO, Configs.Keys.HEAP_RESERVED_MIN_RATIO
);

Expand Down Expand Up @@ -137,7 +133,7 @@
List<? super AutoCloseable> closeableResources,
MasterEnvironment masterEnv, MasterEnvironmentContext masterEnvContext,
EnvironmentOptions options) {
CConfiguration cConf = injector.getInstance(CConfiguration.class);

Check warning on line 136 in cdap-master/src/main/java/io/cdap/cdap/master/environment/k8s/PreviewServiceMain.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'cConf' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).

Check warning on line 136 in cdap-master/src/main/java/io/cdap/cdap/master/environment/k8s/PreviewServiceMain.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck 

Distance between variable 'cConf' declaration and its first usage is 4, but allowed 3.  Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value).
services.add(new TwillRunnerServiceWrapper(injector.getInstance(TwillRunnerService.class)));
services.add(injector.getInstance(PreviewHttpServer.class));
Binding<ZKClientService> zkBinding = injector.getExistingBinding(
Expand Down
Loading