XComp commented on a change in pull request #13004:
URL: https://github.com/apache/flink/pull/13004#discussion_r462269930
##########
File path: flink-core/src/main/java/org/apache/flink/util/ExceptionUtils.java
##########
@@ -349,6 +349,18 @@ public static void tryRethrowException(@Nullable Exception
e) throws Exception {
}
}
+ /**
+ * Tries to throw the given throwable if not null.
+ *
+ * @param t throwable to throw if not null
+ * @throws Throwable
+ */
+ public static void tryRethrowThrowable(@Nullable Throwable t) throws
Throwable {
Review comment:
We could combine `tryRethrowThrowable(Throwable)` and
`tryRethrowException(Exception)`:
```suggestion
public static <T extends Throwable> void tryRethrowThrowable(@Nullable
T t) throws T {
if (t != null) {
throw t;
}
}
```
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/config/memory/JvmMetaspaceAndOverhead.java
##########
@@ -49,4 +50,16 @@ public MemorySize getMetaspace() {
public MemorySize getOverhead() {
return overhead;
}
+
+ @Override
+ public boolean equals(Object obj) {
Review comment:
Providing a customized `equals(Object)` usually means that one should
also provide a corresponding `hashCode()` implementation. Is the `hashCode()`
method missing on purpose?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/config/memory/jobmanager/JobManagerFlinkMemory.java
##########
@@ -50,7 +53,8 @@
private final MemorySize jvmHeap;
private final MemorySize offHeapMemory;
- JobManagerFlinkMemory(MemorySize jvmHeap, MemorySize offHeapMemory) {
+ @VisibleForTesting
Review comment:
Why do we use the `@VisibleForTesting` annotation here? It looks like
the constructor is also used in the context of production code.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/config/memory/jobmanager/JobManagerFlinkMemory.java
##########
@@ -69,4 +73,16 @@ public MemorySize getJvmDirectMemorySize() {
public MemorySize getTotalFlinkMemorySize() {
return jvmHeap.add(offHeapMemory);
}
+
+ @Override
+ public boolean equals(Object obj) {
Review comment:
Providing a customized `equals(Object)` usually means that one should
also provide a corresponding `hashCode()` implementation. Is the `hashCode()`
method missing on purpose?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/config/memory/CommonProcessMemorySpec.java
##########
@@ -94,4 +96,16 @@ public MemorySize getTotalFlinkMemorySize() {
public MemorySize getTotalProcessMemorySize() {
return
flinkMemory.getTotalFlinkMemorySize().add(getJvmMetaspaceSize()).add(getJvmOverheadSize());
}
+
+ @Override
+ public boolean equals(Object obj) {
Review comment:
Providing a customized `equals(Object)` usually means that one should
also provide a corresponding `hashCode()` implementation. Is the `hashCode()`
method missing on purpose?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/config/memory/taskmanager/TaskExecutorFlinkMemory.java
##########
@@ -128,4 +130,20 @@ public MemorySize getJvmDirectMemorySize() {
public MemorySize getTotalFlinkMemorySize() {
return
frameworkHeap.add(frameworkOffHeap).add(taskHeap).add(taskOffHeap).add(network).add(managed);
}
+
+ @Override
+ public boolean equals(Object obj) {
Review comment:
Providing a customized `equals(Object)` usually means that one should
also provide a corresponding `hashCode()` implementation. Is the `hashCode()`
method missing on purpose?
----------------------------------------------------------------
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]
