xintongsong commented on a change in pull request #18148:
URL: https://github.com/apache/flink/pull/18148#discussion_r774852076
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/ThreadDumpInfo.java
##########
@@ -34,17 +35,25 @@
@JsonProperty(FIELD_NAME_THREAD_INFOS)
private final Collection<ThreadInfo> threadInfos;
- private ThreadDumpInfo(Collection<ThreadInfo> threadInfos) {
+ @JsonCreator
+ public ThreadDumpInfo(
+ @JsonProperty(FIELD_NAME_THREAD_INFOS) Collection<ThreadInfo>
threadInfos) {
this.threadInfos = threadInfos;
}
public Collection<ThreadInfo> getThreadInfos() {
return threadInfos;
}
- @JsonCreator
- public static ThreadDumpInfo create(
- @JsonProperty(FIELD_NAME_THREAD_INFOS) Collection<ThreadInfo>
threadInfos) {
+ public static ThreadDumpInfo
create(Collection<java.lang.management.ThreadInfo> threadDump) {
+ final Collection<ThreadDumpInfo.ThreadInfo> threadInfos =
+ threadDump.stream()
+ .map(
+ threadInfo ->
+ ThreadDumpInfo.ThreadInfo.create(
+ threadInfo.getThreadName(),
threadInfo.toString()))
+ .collect(Collectors.toList());
+
Review comment:
1. It would be a good practice to stick to either constructors or
factory methods for a class. Having both public constructor and factory methods
could be confusing in long term.
2. When I suggested deduplicating with a static method in my earlier
comment, I meant to deduplicate the logic "take a thread dump", which is not an
exact replacement of a constructor which "create a ThreadDumpInfo with given
thread information".
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
