Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2800#discussion_r210632176
--- Diff: storm-client/src/jvm/org/apache/storm/stats/StatsUtil.java ---
@@ -1525,27 +1528,24 @@ public static ComponentPageInfo aggCompExecsStats(
* @param timeout timeout
* @return a HashMap of updated executor heart beats
*/
- public static Map<List<Integer>, Map<String, Object>>
updateHeartbeatCacheFromZkHeartbeat(Map<List<Integer>, Map<String, Object>>
cache,
-
Map<List<Integer>, Map<String, Object>>
-
executorBeats,
-
Set<List<Integer>> executors,
-
Integer timeout) {
- Map<List<Integer>, Map<String, Object>> ret = new HashMap<>();
- if (cache == null && executorBeats == null) {
- return ret;
- }
-
+ public static ConcurrentMap<List<Integer>, Map<String, Object>>
updateHeartbeatCacheFromZkHeartbeat(Map<List<Integer>, Map<String, Object>>
cache,
+
Map<List<Integer>, Map<String, Object>>
+
executorBeats,
+
Set<List<Integer>> executors,
+
Integer timeout) {
if (cache == null) {
+ if (executorBeats == null) {
--- End diff --
The construction here seems a little odd. Initializing cache and
executorBeats when null isn't necessary. I think we should keep the "if cache
and executorBeats are null" clause, then replace the other two branches with
something like `Map<String, Object> currBeat = cache == null ? null :
cache.get(executor);` and equivalent for `newBeat`.
---
