zhuzhurk commented on a change in pull request #10179: [FLINK-14734][runtime]
Add a ResourceSpec in SlotSharingGroup to describe its overall resources
URL: https://github.com/apache/flink/pull/10179#discussion_r346921259
##########
File path:
flink-core/src/main/java/org/apache/flink/api/common/operators/ResourceSpec.java
##########
@@ -149,6 +150,39 @@ public ResourceSpec merge(final ResourceSpec other) {
return target;
}
+ /**
+ * Subtracts another resource spec from this one.
+ *
+ * @param other The other resource spec to subtract.
+ * @return The subtracted resource spec.
+ */
+ public ResourceSpec subtract(final ResourceSpec other) {
+ checkNotNull(other, "Cannot subtract null resources");
+
+ if (this.equals(UNKNOWN) || other.equals(UNKNOWN)) {
+ return UNKNOWN;
+ }
+
+ checkArgument(other.lessThanOrEqual(this), "Cannot subtract a
larger ResourceSpec from this one.");
+
+ final ResourceSpec target = new ResourceSpec(
+ this.cpuCores.subtract(other.cpuCores),
+ this.taskHeapMemory.subtract(other.taskHeapMemory),
+
this.taskOffHeapMemory.subtract(other.taskOffHeapMemory),
+
this.onHeapManagedMemory.subtract(other.onHeapManagedMemory),
+
this.offHeapManagedMemory.subtract(other.offHeapManagedMemory));
+
+ target.extendedResources.putAll(extendedResources);
+
+ for (Resource resource : other.extendedResources.values()) {
+ target.extendedResources.merge(resource.getName(),
resource, (v1, v2) -> {
+ final Resource subtracted = v1.subtract(v2);
+ return
subtracted.getValue().compareTo(BigDecimal.ZERO) == 0 ? null : subtracted;
Review comment:
It's a good question. Looks we need to consider how we should treat a value
0 Resource.
CPUResource with value 0 is used in production currently, thus Resource must
accept value 0. And CPUResource would not be null in
ResourceSpec/ResourceProfile (except for the special UNKNOWN case where null
means unknown value).
However, 0 value extends Resource is a bit confusing, since 0 and null also
mean no such resource. If accepting both of them, we need to consider it in all
comparisons(equals/lessThanOrEqual/isMatch) and aggregations(merge/subtract).
To be simple, I think we can change it like this: in
`ResourceSpec`/`ResourceProfile` we only keep extended resource with positive
values. If it is 0 value, we need to drop it so the resource would always be
null. This change is needed in the constructors and `subtract()`.
Any usages of extended resources, must check whether it's null to see
whether that extended resource is needed(for task)/available(in slot).
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
