Skip to content

Commit 108a3c5

Browse files
axreldablecodope
authored andcommitted
[java] encapsulation + resource immutability for option classes (ray-project#54370)
Signed-off-by: axreldable <[email protected]> Co-authored-by: Sagar Sumit <[email protected]> Signed-off-by: dshepelev15 <[email protected]>
1 parent d002b86 commit 108a3c5

File tree

8 files changed

+305
-156
lines changed

8 files changed

+305
-156
lines changed

java/api/src/main/java/io/ray/api/options/ActorCreationOptions.java

Lines changed: 99 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,58 +14,107 @@ public class ActorCreationOptions extends BaseTaskOptions {
1414
public static final int NO_RESTART = 0;
1515
public static final int INFINITE_RESTART = -1;
1616

17-
public final String name;
18-
public ActorLifetime lifetime;
19-
public final int maxRestarts;
20-
public final int maxTaskRetries;
21-
public final List<String> jvmOptions;
22-
public final int maxConcurrency;
23-
public final PlacementGroup group;
24-
public final int bundleIndex;
25-
public final List<ConcurrencyGroup> concurrencyGroups;
26-
public final String serializedRuntimeEnv;
27-
public final String namespace;
28-
public final int maxPendingCalls;
29-
public final boolean isAsync;
30-
public final boolean executeOutOfOrder;
31-
32-
private ActorCreationOptions(
33-
String name,
34-
ActorLifetime lifetime,
35-
Map<String, Double> resources,
36-
int maxRestarts,
37-
int maxTaskRetries,
38-
List<String> jvmOptions,
39-
int maxConcurrency,
40-
PlacementGroup group,
41-
int bundleIndex,
42-
List<ConcurrencyGroup> concurrencyGroups,
43-
String serializedRuntimeEnv,
44-
String namespace,
45-
int maxPendingCalls,
46-
boolean isAsync) {
47-
super(resources);
48-
this.name = name;
49-
this.lifetime = lifetime;
50-
this.maxRestarts = maxRestarts;
51-
this.maxTaskRetries = maxTaskRetries;
52-
this.jvmOptions = jvmOptions;
53-
this.maxConcurrency = maxConcurrency;
54-
this.group = group;
55-
this.bundleIndex = bundleIndex;
56-
this.concurrencyGroups = concurrencyGroups;
57-
this.serializedRuntimeEnv = serializedRuntimeEnv;
58-
this.namespace = namespace;
59-
this.maxPendingCalls = maxPendingCalls;
60-
this.isAsync = isAsync;
61-
this.executeOutOfOrder = isAsync;
17+
private final String name;
18+
private final ActorLifetime lifetime;
19+
private final int maxRestarts;
20+
private final int maxTaskRetries;
21+
private final List<String> jvmOptions;
22+
private final int maxConcurrency;
23+
private final PlacementGroup group;
24+
private final int bundleIndex;
25+
private final List<ConcurrencyGroup> concurrencyGroups;
26+
private final String serializedRuntimeEnv;
27+
private final String namespace;
28+
private final int maxPendingCalls;
29+
private final boolean isAsync;
30+
private final boolean executeOutOfOrder;
31+
32+
private ActorCreationOptions(Builder builder) {
33+
super(builder.resources);
34+
this.name = builder.name;
35+
this.lifetime = builder.lifetime;
36+
this.maxRestarts = builder.maxRestarts;
37+
this.maxTaskRetries = builder.maxTaskRetries;
38+
this.jvmOptions =
39+
builder.jvmOptions != null
40+
? java.util.Collections.unmodifiableList(builder.jvmOptions)
41+
: null;
42+
this.maxConcurrency = builder.maxConcurrency;
43+
this.group = builder.group;
44+
this.bundleIndex = builder.bundleIndex;
45+
this.concurrencyGroups =
46+
builder.concurrencyGroups != null
47+
? java.util.Collections.unmodifiableList(builder.concurrencyGroups)
48+
: null;
49+
this.serializedRuntimeEnv =
50+
builder.runtimeEnv != null ? builder.runtimeEnv.serializeToRuntimeEnvInfo() : "";
51+
this.namespace = builder.namespace;
52+
this.maxPendingCalls = builder.maxPendingCalls;
53+
this.isAsync = builder.isAsync;
54+
this.executeOutOfOrder = builder.isAsync;
55+
}
56+
57+
public String getName() {
58+
return name;
59+
}
60+
61+
public ActorLifetime getLifetime() {
62+
return lifetime;
63+
}
64+
65+
public int getMaxRestarts() {
66+
return maxRestarts;
67+
}
68+
69+
public int getMaxTaskRetries() {
70+
return maxTaskRetries;
71+
}
72+
73+
public List<String> getJvmOptions() {
74+
return jvmOptions;
75+
}
76+
77+
public int getMaxConcurrency() {
78+
return maxConcurrency;
79+
}
80+
81+
public PlacementGroup getGroup() {
82+
return group;
83+
}
84+
85+
public int getBundleIndex() {
86+
return bundleIndex;
87+
}
88+
89+
public List<ConcurrencyGroup> getConcurrencyGroups() {
90+
return concurrencyGroups;
91+
}
92+
93+
public String getSerializedRuntimeEnv() {
94+
return serializedRuntimeEnv;
95+
}
96+
97+
public String getNamespace() {
98+
return namespace;
99+
}
100+
101+
public int getMaxPendingCalls() {
102+
return maxPendingCalls;
103+
}
104+
105+
public boolean isAsync() {
106+
return isAsync;
107+
}
108+
109+
public boolean isExecuteOutOfOrder() {
110+
return executeOutOfOrder;
62111
}
63112

64113
/** The inner class for building ActorCreationOptions. */
65114
public static class Builder {
66115
private String name;
67116
private ActorLifetime lifetime = null;
68-
private Map<String, Double> resources = new HashMap<>();
117+
private final Map<String, Double> resources = new HashMap<>();
69118
private int maxRestarts = 0;
70119
private int maxTaskRetries = 0;
71120
private List<String> jvmOptions = new ArrayList<>();
@@ -223,24 +272,6 @@ public Builder setPlacementGroup(PlacementGroup group, int bundleIndex) {
223272
return this;
224273
}
225274

226-
public ActorCreationOptions build() {
227-
return new ActorCreationOptions(
228-
name,
229-
lifetime,
230-
resources,
231-
maxRestarts,
232-
maxTaskRetries,
233-
jvmOptions,
234-
maxConcurrency,
235-
group,
236-
bundleIndex,
237-
concurrencyGroups,
238-
runtimeEnv != null ? runtimeEnv.serializeToRuntimeEnvInfo() : "",
239-
namespace,
240-
maxPendingCalls,
241-
isAsync);
242-
}
243-
244275
/** Set the concurrency groups for this actor. */
245276
public Builder setConcurrencyGroups(List<ConcurrencyGroup> concurrencyGroups) {
246277
this.concurrencyGroups = concurrencyGroups;
@@ -256,5 +287,9 @@ public Builder setNamespace(String namespace) {
256287
this.namespace = namespace;
257288
return this;
258289
}
290+
291+
public ActorCreationOptions build() {
292+
return new ActorCreationOptions(this);
293+
}
259294
}
260295
}
Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,70 @@
11
package io.ray.api.options;
22

33
import java.io.Serializable;
4+
import java.util.Collections;
45
import java.util.HashMap;
56
import java.util.Map;
67

78
/** The options class for RayCall or ActorCreation. */
89
public abstract class BaseTaskOptions implements Serializable {
910

10-
public final Map<String, Double> resources = new HashMap<>();
11-
12-
public BaseTaskOptions() {}
11+
private final Map<String, Double> resources;
1312

1413
public BaseTaskOptions(Map<String, Double> resources) {
14+
if (resources == null) {
15+
throw new IllegalArgumentException("Resources map should not be null!");
16+
}
17+
18+
Map<String, Double> filteredResources = validateAndFilterResources(resources);
19+
this.resources = Collections.unmodifiableMap(filteredResources);
20+
}
21+
22+
private Map<String, Double> validateAndFilterResources(Map<String, Double> resources) {
23+
Map<String, Double> filtered = new HashMap<>();
1524
for (Map.Entry<String, Double> entry : resources.entrySet()) {
16-
if (entry.getValue() == null || entry.getValue().compareTo(0.0) < 0) {
17-
throw new IllegalArgumentException(
18-
String.format(
19-
"Resource values should be " + "non negative. Specified resource: %s = %s.",
20-
entry.getKey(), entry.getValue()));
21-
}
22-
// Note: A resource value should be an integer if it is greater than 1.0.
23-
// e.g. 3.0 is a valid resource value, but 3.5 is not.
24-
if (entry.getValue().compareTo(1.0) >= 0
25-
&& entry.getValue().compareTo(Math.floor(entry.getValue())) != 0) {
26-
throw new IllegalArgumentException(
27-
String.format(
28-
"A resource value should be "
29-
+ "an integer if it is greater than 1.0. Specified resource: %s = %s.",
30-
entry.getKey(), entry.getValue()));
25+
String name = entry.getKey();
26+
Double value = entry.getValue();
27+
28+
validateResourceValue(name, value);
29+
validateIntegerConstraint(name, value);
30+
31+
if (value != 0.0) {
32+
filtered.put(name, value);
3133
}
3234
}
33-
/// Filter 0 resources
34-
resources.forEach(
35-
(key, value) -> {
36-
if (value != 0) {
37-
this.resources.put(key, value);
38-
}
39-
});
35+
return filtered;
36+
}
37+
38+
private void validateResourceValue(String name, Double value) {
39+
if (name == null || value == null) {
40+
throw new IllegalArgumentException(
41+
String.format(
42+
"Resource name and value should not be null. Specified resource: %s = %s.",
43+
name, value));
44+
} else if (value < 0.0) {
45+
throw new IllegalArgumentException(
46+
String.format(
47+
"Resource values should be non negative. Specified resource: %s = %s.", name, value));
48+
}
49+
}
50+
51+
/**
52+
* Validates that resource values greater than or equal to 1.0 are integers.
53+
*
54+
* @param name the name of the resource being validated
55+
* @param value the value of the resource to validate
56+
* @throws IllegalArgumentException if the value is >= 1.0 and not an integer
57+
*/
58+
private void validateIntegerConstraint(String name, Double value) {
59+
if (value >= 1.0 && value % 1.0 != 0.0) {
60+
throw new IllegalArgumentException(
61+
String.format(
62+
"A resource value should be an integer if it is greater than 1.0. Specified resource: %s = %s",
63+
name, value));
64+
}
65+
}
66+
67+
public Map<String, Double> getResources() {
68+
return resources;
4069
}
4170
}

java/api/src/main/java/io/ray/api/options/CallOptions.java

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,40 @@
88
/** The options for RayCall. */
99
public class CallOptions extends BaseTaskOptions {
1010

11-
public final String name;
12-
public final PlacementGroup group;
13-
public final int bundleIndex;
14-
public final String concurrencyGroupName;
11+
private final String name;
12+
private final PlacementGroup group;
13+
private final int bundleIndex;
14+
private final String concurrencyGroupName;
1515
private final String serializedRuntimeEnvInfo;
1616

17-
private CallOptions(
18-
String name,
19-
Map<String, Double> resources,
20-
PlacementGroup group,
21-
int bundleIndex,
22-
String concurrencyGroupName,
23-
RuntimeEnv runtimeEnv) {
24-
super(resources);
25-
this.name = name;
26-
this.group = group;
27-
this.bundleIndex = bundleIndex;
28-
this.concurrencyGroupName = concurrencyGroupName;
17+
private CallOptions(Builder builder) {
18+
super(builder.resources);
19+
this.name = builder.name;
20+
this.group = builder.group;
21+
this.bundleIndex = builder.bundleIndex;
22+
this.concurrencyGroupName = builder.concurrencyGroupName;
2923
this.serializedRuntimeEnvInfo =
30-
runtimeEnv == null ? "" : runtimeEnv.serializeToRuntimeEnvInfo();
24+
builder.runtimeEnv == null ? "" : builder.runtimeEnv.serializeToRuntimeEnvInfo();
25+
}
26+
27+
public String getName() {
28+
return name;
29+
}
30+
31+
public PlacementGroup getGroup() {
32+
return group;
33+
}
34+
35+
public int getBundleIndex() {
36+
return bundleIndex;
37+
}
38+
39+
public String getConcurrencyGroupName() {
40+
return concurrencyGroupName;
41+
}
42+
43+
public String getSerializedRuntimeEnvInfo() {
44+
return serializedRuntimeEnvInfo;
3145
}
3246

3347
/** This inner class for building CallOptions. */
@@ -100,7 +114,7 @@ public Builder setRuntimeEnv(RuntimeEnv runtimeEnv) {
100114
}
101115

102116
public CallOptions build() {
103-
return new CallOptions(name, resources, group, bundleIndex, concurrencyGroupName, runtimeEnv);
117+
return new CallOptions(this);
104118
}
105119
}
106120
}

0 commit comments

Comments
 (0)