YARN-3276. Code cleanup for timeline service API records. Contributed by Junping Du.

(cherry picked from commit d88f30ba5359f59fb71b93a55e1c1d9a1c0dff8e)

hadoop-yarn-project/CHANGES.txt

@@ -84,6 +84,9 @@ Branch YARN-2928: Timeline Server Next Generation: Phase 1
 
   IMPROVEMENTS
 
+    YARN-3276. Code cleanup for timeline service API records. (Junping Du via
+    zjshen)
+
   OPTIMIZATIONS
 
   BUG FIXES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEntity.java

@@ -34,6 +34,7 @@ import javax.xml.bind.annotation.XmlRootElement;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Evolving;
+import org.apache.hadoop.yarn.util.TimelineServiceHelper;
 
 /**
  * <p>
@@ -231,11 +232,8 @@ public class TimelineEntity implements Comparable<TimelineEntity> {
    */
   public void setRelatedEntities(
       Map<String, Set<String>> relatedEntities) {
-    if (relatedEntities != null && !(relatedEntities instanceof HashMap)) {
-      this.relatedEntities = new HashMap<String, Set<String>>(relatedEntities);
-    } else {
-      this.relatedEntities = (HashMap<String, Set<String>>) relatedEntities;
-    }
+    this.relatedEntities = TimelineServiceHelper.mapCastToHashMap(
+        relatedEntities);
   }
 
   /**
@@ -297,11 +295,8 @@ public class TimelineEntity implements Comparable<TimelineEntity> {
    *          a map of primary filters
    */
   public void setPrimaryFilters(Map<String, Set<Object>> primaryFilters) {
-    if (primaryFilters != null && !(primaryFilters instanceof HashMap)) {
-      this.primaryFilters = new HashMap<String, Set<Object>>(primaryFilters);
-    } else {
-      this.primaryFilters = (HashMap<String, Set<Object>>) primaryFilters;
-    }
+    this.primaryFilters =
+        TimelineServiceHelper.mapCastToHashMap(primaryFilters);
   }
 
   /**
@@ -350,11 +345,7 @@ public class TimelineEntity implements Comparable<TimelineEntity> {
    *          a map of other information
    */
   public void setOtherInfo(Map<String, Object> otherInfo) {
-    if (otherInfo != null && !(otherInfo instanceof HashMap)) {
-      this.otherInfo = new HashMap<String, Object>(otherInfo);
-    } else {
-      this.otherInfo = (HashMap<String, Object>) otherInfo;
-    }
+    this.otherInfo = TimelineServiceHelper.mapCastToHashMap(otherInfo);
   }
 
   /**

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEvent.java

@@ -29,6 +29,7 @@ import javax.xml.bind.annotation.XmlRootElement;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Evolving;
+import org.apache.hadoop.yarn.util.TimelineServiceHelper;
 
 /**
  * The class that contains the information of an event that is related to some
@@ -135,11 +136,8 @@ public class TimelineEvent implements Comparable<TimelineEvent> {
    *          a map of of the information of the event
    */
   public void setEventInfo(Map<String, Object> eventInfo) {
-    if (eventInfo != null && !(eventInfo instanceof HashMap)) {
-      this.eventInfo = new HashMap<String, Object>(eventInfo);
-    } else {
-      this.eventInfo = (HashMap<String, Object>) eventInfo;
-    }
+    this.eventInfo = TimelineServiceHelper.mapCastToHashMap(
+        eventInfo);
   }
 
   @Override

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java

@@ -19,6 +19,7 @@ package org.apache.hadoop.yarn.api.records.timelineservice;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.yarn.util.TimelineServiceHelper;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -184,11 +185,7 @@ public class TimelineEntity {
 
   public void setInfo(Map<String, Object> info) {
     if (real == null) {
-      if (info != null && !(info instanceof HashMap)) {
-        this.info = new HashMap<String, Object>(info);
-      } else {
-        this.info = (HashMap<String, Object>) info;
-      }
+      this.info = TimelineServiceHelper.mapCastToHashMap(info);
     } else {
       real.setInfo(info);
     }
@@ -231,11 +228,7 @@ public class TimelineEntity {
 
   public void setConfigs(Map<String, String> configs) {
     if (real == null) {
-      if (configs != null && !(configs instanceof HashMap)) {
-        this.configs = new HashMap<String, String>(configs);
-      } else {
-        this.configs = (HashMap<String, String>) configs;
-      }
+      this.configs = TimelineServiceHelper.mapCastToHashMap(configs);
     } else {
       real.setConfigs(configs);
     }
@@ -345,14 +338,8 @@ public class TimelineEntity {
   public void setIsRelatedToEntities(
       Map<String, Set<String>> isRelatedToEntities) {
     if (real == null) {
-      if (isRelatedToEntities != null &&
-          !(isRelatedToEntities instanceof HashMap)) {
-        this.isRelatedToEntities =
-            new HashMap<String, Set<String>>(isRelatedToEntities);
-      } else {
-        this.isRelatedToEntities =
-            (HashMap<String, Set<String>>) isRelatedToEntities;
-      }
+      this.isRelatedToEntities =
+          TimelineServiceHelper.mapCastToHashMap(isRelatedToEntities);
     } else {
       real.setIsRelatedToEntities(isRelatedToEntities);
     }
@@ -438,14 +425,8 @@ public class TimelineEntity {
 
   public void setRelatesToEntities(Map<String, Set<String>> relatesToEntities) {
     if (real == null) {
-      if (relatesToEntities != null &&
-          !(relatesToEntities instanceof HashMap)) {
-        this.relatesToEntities =
-            new HashMap<String, Set<String>>(relatesToEntities);
-      } else {
-        this.relatesToEntities =
-            (HashMap<String, Set<String>>) relatesToEntities;
-      }
+      this.relatesToEntities =
+          TimelineServiceHelper.mapCastToHashMap(relatesToEntities);
     } else {
       real.setRelatesToEntities(relatesToEntities);
     }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java

@@ -19,6 +19,7 @@ package org.apache.hadoop.yarn.api.records.timelineservice;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.yarn.util.TimelineServiceHelper;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -61,11 +62,7 @@ public class TimelineEvent {
   }
 
   public void setInfo(Map<String, Object> info) {
-    if (info != null && !(info instanceof HashMap)) {
-      this.info = new HashMap<String, Object>(info);
-    } else {
-      this.info = (HashMap<String, Object>) info;
-    }
+    this.info = TimelineServiceHelper.mapCastToHashMap(info);
   }
 
   public void addInfo(Map<String, Object> info) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java

@@ -44,7 +44,7 @@ public class TimelineMetric {
   private Comparator<Long> reverseComparator = new Comparator<Long>() {
     @Override
     public int compare(Long l1, Long l2) {
-      return -l1.compareTo(l2);
+      return l2.compareTo(l1);
     }
   };
   private TreeMap<Long, Number> values = new TreeMap<>(reverseComparator);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/TimelineServiceHelper.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.util;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.hadoop.classification.InterfaceAudience.LimitedPrivate;
+
+/**
+ * Helper class for Timeline service.
+ */
+@LimitedPrivate({ "MapReduce", "YARN" })
+public final class TimelineServiceHelper {
+
+  private TimelineServiceHelper() {
+    // Utility classes should not have a public or default constructor.
+  }
+
+  /**
+   * Cast map to HashMap for generic type.
+   * @param originalMap the map need to be casted
+   * @return casted HashMap object
+   */
+  public static <E, V> HashMap<E, V> mapCastToHashMap(
+      Map<E, V> originalMap) {
+    return originalMap == null ? null : originalMap instanceof HashMap ?
+        (HashMap<E, V>) originalMap : new HashMap<E, V>(originalMap);
+  }
+
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java

@@ -387,13 +387,13 @@ public class AllocateResponsePBImpl extends AllocateResponse {
 
 
   @Override
-  public String getCollectorAddr() {
+  public synchronized String getCollectorAddr() {
     AllocateResponseProtoOrBuilder p = viaProto ? proto : builder;
     return p.getCollectorAddr();
   }
 
   @Override
-  public void setCollectorAddr(String collectorAddr) {
+  public synchronized void setCollectorAddr(String collectorAddr) {
     maybeInitBuilder();
     if (collectorAddr == null) {
       builder.clearCollectorAddr();

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestTimelineServiceHelper.java

@@ -0,0 +1,76 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.yarn.util;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestTimelineServiceHelper {
+  
+  @Test
+  public void testMapCastToHashMap() {
+
+    // Test null map be casted to null
+    Map<String, String> nullMap = null;
+    Assert.assertNull(TimelineServiceHelper.mapCastToHashMap(nullMap));
+    
+    // Test empty hashmap be casted to a empty hashmap
+    Map<String, String> emptyHashMap = new HashMap<String, String>();
+    Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(emptyHashMap).size(), 0);
+    
+    // Test empty non-hashmap be casted to a empty hashmap
+    Map<String, String> emptyTreeMap = new TreeMap<String, String>();
+    Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(emptyTreeMap).size(), 0);
+    
+    // Test non-empty hashmap be casted to hashmap correctly
+    Map<String, String> firstHashMap = new HashMap<String, String>();
+    String key = "KEY";
+    String value = "VALUE";
+    firstHashMap.put(key, value);
+    Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(firstHashMap), firstHashMap);
+    
+    // Test non-empty non-hashmap is casted correctly.
+    Map<String, String> firstTreeMap = new TreeMap<String, String>();
+    firstTreeMap.put(key, value);
+    HashMap<String, String> alternateHashMap = 
+        TimelineServiceHelper.mapCastToHashMap(firstTreeMap);
+    Assert.assertEquals(firstTreeMap.size(), alternateHashMap.size());
+    Assert.assertEquals(alternateHashMap.get(key), value);
+    
+    // Test complicated hashmap be casted correctly
+    Map<String, Set<String>> complicatedHashMap = new HashMap<String, Set<String>>();
+    Set<String> hashSet = new HashSet<String>();
+    hashSet.add(value);
+    complicatedHashMap.put(key, hashSet);
+    Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(complicatedHashMap), 
+        complicatedHashMap);
+    
+    // Test complicated non-hashmap get casted correctly
+    Map<String, Set<String>> complicatedTreeMap = new TreeMap<String, Set<String>>();
+    complicatedTreeMap.put(key, hashSet);
+    Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(complicatedTreeMap).get(key), 
+        hashSet);
+  }
+
+}