tillrohrmann commented on a change in pull request #11916:
URL: https://github.com/apache/flink/pull/11916#discussion_r415581488
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/WorkerSpecContainerResourceAdapter.java
##########
@@ -128,12 +142,61 @@ private int normalize(final int value, final int
unitValue) {
return MathUtils.divideRoundUp(value, unitValue) * unitValue;
}
- boolean resourceWithinMaxAllocation(final Resource resource) {
- return resource.getMemory() <= maxMemMB &&
resource.getVirtualCores() <= maxVcore;
+ boolean resourceWithinMaxAllocation(final InternalContainerResource
resource) {
+ return resource.memory <= maxMemMB && resource.vcores <=
maxVcore;
}
enum MatchingStrategy {
MATCH_VCORE,
IGNORE_VCORE
}
+
+ /**
+ * An {@link InternalContainerResource} corresponds to a {@link
Resource}.
+ * This class is for {@link WorkerSpecContainerResourceAdapter}
internal usages only, to overcome the problem that
+ * hash codes are calculated inconsistently across different {@link
Resource} implementations.
+ */
+ private class InternalContainerResource {
+ private final int memory;
+ private final int vcores;
+
+ private InternalContainerResource(final int memory, final int
vcores) {
+ this.memory = memory;
+ this.vcores = vcores;
+ }
+
+ private InternalContainerResource(final Resource resource) {
+ this(
+
Preconditions.checkNotNull(resource).getMemory(),
+
Preconditions.checkNotNull(resource).getVirtualCores());
+ }
+
+ private Resource toResource() {
+ return Resource.newInstance(memory, vcores);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == this) {
+ return true;
+ } else if (obj != null && obj.getClass() ==
InternalContainerResource.class) {
+ InternalContainerResource that =
(InternalContainerResource) obj;
+ return Objects.equals(this.memory, that.memory)
&&
+ Objects.equals(this.vcores,
that.vcores);
Review comment:
nit: this will cause auto boxing and unboxing. we could simply use
`memory == that.memory`.
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/WorkerSpecContainerResourceAdapter.java
##########
@@ -128,12 +142,61 @@ private int normalize(final int value, final int
unitValue) {
return MathUtils.divideRoundUp(value, unitValue) * unitValue;
}
- boolean resourceWithinMaxAllocation(final Resource resource) {
- return resource.getMemory() <= maxMemMB &&
resource.getVirtualCores() <= maxVcore;
+ boolean resourceWithinMaxAllocation(final InternalContainerResource
resource) {
+ return resource.memory <= maxMemMB && resource.vcores <=
maxVcore;
}
enum MatchingStrategy {
MATCH_VCORE,
IGNORE_VCORE
}
+
+ /**
+ * An {@link InternalContainerResource} corresponds to a {@link
Resource}.
+ * This class is for {@link WorkerSpecContainerResourceAdapter}
internal usages only, to overcome the problem that
+ * hash codes are calculated inconsistently across different {@link
Resource} implementations.
+ */
+ private class InternalContainerResource {
+ private final int memory;
+ private final int vcores;
+
+ private InternalContainerResource(final int memory, final int
vcores) {
+ this.memory = memory;
+ this.vcores = vcores;
+ }
+
+ private InternalContainerResource(final Resource resource) {
+ this(
+
Preconditions.checkNotNull(resource).getMemory(),
+
Preconditions.checkNotNull(resource).getVirtualCores());
+ }
+
+ private Resource toResource() {
+ return Resource.newInstance(memory, vcores);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == this) {
+ return true;
+ } else if (obj != null && obj.getClass() ==
InternalContainerResource.class) {
+ InternalContainerResource that =
(InternalContainerResource) obj;
+ return Objects.equals(this.memory, that.memory)
&&
+ Objects.equals(this.vcores,
that.vcores);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hashCode(memory);
+ result = 31 * result + Objects.hashCode(vcores);
Review comment:
Also here. The idiomatic way to create the hash code for a primitive is
to call `XYZ.hashCode(value)` where `XYZ` is the class of the primitive type.
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/WorkerSpecContainerResourceAdapter.java
##########
@@ -128,12 +142,61 @@ private int normalize(final int value, final int
unitValue) {
return MathUtils.divideRoundUp(value, unitValue) * unitValue;
}
- boolean resourceWithinMaxAllocation(final Resource resource) {
- return resource.getMemory() <= maxMemMB &&
resource.getVirtualCores() <= maxVcore;
+ boolean resourceWithinMaxAllocation(final InternalContainerResource
resource) {
+ return resource.memory <= maxMemMB && resource.vcores <=
maxVcore;
}
enum MatchingStrategy {
MATCH_VCORE,
IGNORE_VCORE
}
+
+ /**
+ * An {@link InternalContainerResource} corresponds to a {@link
Resource}.
+ * This class is for {@link WorkerSpecContainerResourceAdapter}
internal usages only, to overcome the problem that
+ * hash codes are calculated inconsistently across different {@link
Resource} implementations.
+ */
+ private class InternalContainerResource {
+ private final int memory;
+ private final int vcores;
+
+ private InternalContainerResource(final int memory, final int
vcores) {
+ this.memory = memory;
+ this.vcores = vcores;
+ }
+
+ private InternalContainerResource(final Resource resource) {
+ this(
+
Preconditions.checkNotNull(resource).getMemory(),
+
Preconditions.checkNotNull(resource).getVirtualCores());
+ }
+
+ private Resource toResource() {
+ return Resource.newInstance(memory, vcores);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == this) {
+ return true;
+ } else if (obj != null && obj.getClass() ==
InternalContainerResource.class) {
Review comment:
`final` sounds good
----------------------------------------------------------------
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:
us...@infra.apache.org
