From 42d7864b84e42fafb529f9920c6633ff59c55217 Mon Sep 17 00:00:00 2001 From: Zhijie Shen Date: Wed, 3 Jun 2015 15:13:29 -0700 Subject: [PATCH] YARN-3276. Code cleanup for timeline service API records. Contributed by Junping Du. (cherry picked from commit d88f30ba5359f59fb71b93a55e1c1d9a1c0dff8e) --- .../api/records/timeline/TimelineEntity.java | 21 ++--- .../api/records/timeline/TimelineEvent.java | 8 +- .../timelineservice/TimelineEntity.java | 33 ++------ .../timelineservice/TimelineEvent.java | 7 +- .../timelineservice/TimelineMetric.java | 2 +- .../yarn/util/TimelineServiceHelper.java | 47 ++++++++++++ .../impl/pb/AllocateResponsePBImpl.java | 4 +- .../yarn/util/TestTimelineServiceHelper.java | 76 +++++++++++++++++++ 8 files changed, 144 insertions(+), 54 deletions(-) create mode 100644 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/TimelineServiceHelper.java create mode 100644 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestTimelineServiceHelper.java diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEntity.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEntity.java index a43259b5ff1..e695050e2b9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEntity.java +++ b/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 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; /** *

@@ -231,11 +232,8 @@ public void addRelatedEntities(Map> relatedEntities) { */ public void setRelatedEntities( Map> relatedEntities) { - if (relatedEntities != null && !(relatedEntities instanceof HashMap)) { - this.relatedEntities = new HashMap>(relatedEntities); - } else { - this.relatedEntities = (HashMap>) relatedEntities; - } + this.relatedEntities = TimelineServiceHelper.mapCastToHashMap( + relatedEntities); } /** @@ -297,11 +295,8 @@ public void addPrimaryFilters(Map> primaryFilters) { * a map of primary filters */ public void setPrimaryFilters(Map> primaryFilters) { - if (primaryFilters != null && !(primaryFilters instanceof HashMap)) { - this.primaryFilters = new HashMap>(primaryFilters); - } else { - this.primaryFilters = (HashMap>) primaryFilters; - } + this.primaryFilters = + TimelineServiceHelper.mapCastToHashMap(primaryFilters); } /** @@ -350,11 +345,7 @@ public void addOtherInfo(Map otherInfo) { * a map of other information */ public void setOtherInfo(Map otherInfo) { - if (otherInfo != null && !(otherInfo instanceof HashMap)) { - this.otherInfo = new HashMap(otherInfo); - } else { - this.otherInfo = (HashMap) otherInfo; - } + this.otherInfo = TimelineServiceHelper.mapCastToHashMap(otherInfo); } /** diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEvent.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEvent.java index 73b2e729c1e..d5611f8da99 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEvent.java +++ b/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 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 void addEventInfo(Map eventInfo) { * a map of of the information of the event */ public void setEventInfo(Map eventInfo) { - if (eventInfo != null && !(eventInfo instanceof HashMap)) { - this.eventInfo = new HashMap(eventInfo); - } else { - this.eventInfo = (HashMap) eventInfo; - } + this.eventInfo = TimelineServiceHelper.mapCastToHashMap( + eventInfo); } @Override diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java index 3be7f52e4a2..defadec4a8e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java @@ -19,6 +19,7 @@ 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 Map getInfo() { public void setInfo(Map info) { if (real == null) { - if (info != null && !(info instanceof HashMap)) { - this.info = new HashMap(info); - } else { - this.info = (HashMap) info; - } + this.info = TimelineServiceHelper.mapCastToHashMap(info); } else { real.setInfo(info); } @@ -231,11 +228,7 @@ public Map getConfigs() { public void setConfigs(Map configs) { if (real == null) { - if (configs != null && !(configs instanceof HashMap)) { - this.configs = new HashMap(configs); - } else { - this.configs = (HashMap) configs; - } + this.configs = TimelineServiceHelper.mapCastToHashMap(configs); } else { real.setConfigs(configs); } @@ -345,14 +338,8 @@ public HashMap> getIsRelatedToEntitiesJAXB() { public void setIsRelatedToEntities( Map> isRelatedToEntities) { if (real == null) { - if (isRelatedToEntities != null && - !(isRelatedToEntities instanceof HashMap)) { - this.isRelatedToEntities = - new HashMap>(isRelatedToEntities); - } else { - this.isRelatedToEntities = - (HashMap>) isRelatedToEntities; - } + this.isRelatedToEntities = + TimelineServiceHelper.mapCastToHashMap(isRelatedToEntities); } else { real.setIsRelatedToEntities(isRelatedToEntities); } @@ -438,14 +425,8 @@ public void addRelatesToEntity(String type, String id) { public void setRelatesToEntities(Map> relatesToEntities) { if (real == null) { - if (relatesToEntities != null && - !(relatesToEntities instanceof HashMap)) { - this.relatesToEntities = - new HashMap>(relatesToEntities); - } else { - this.relatesToEntities = - (HashMap>) relatesToEntities; - } + this.relatesToEntities = + TimelineServiceHelper.mapCastToHashMap(relatesToEntities); } else { real.setRelatesToEntities(relatesToEntities); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java index 517c88c1cb2..b6b82a71cb9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java @@ -19,6 +19,7 @@ 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 Map getInfo() { } public void setInfo(Map info) { - if (info != null && !(info instanceof HashMap)) { - this.info = new HashMap(info); - } else { - this.info = (HashMap) info; - } + this.info = TimelineServiceHelper.mapCastToHashMap(info); } public void addInfo(Map info) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java index c897d39f95f..172b56cda0e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java +++ b/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 static enum Type { private Comparator reverseComparator = new Comparator() { @Override public int compare(Long l1, Long l2) { - return -l1.compareTo(l2); + return l2.compareTo(l1); } }; private TreeMap values = new TreeMap<>(reverseComparator); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/TimelineServiceHelper.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/TimelineServiceHelper.java new file mode 100644 index 00000000000..ff6ebbd712a --- /dev/null +++ b/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 HashMap mapCastToHashMap( + Map originalMap) { + return originalMap == null ? null : originalMap instanceof HashMap ? + (HashMap) originalMap : new HashMap(originalMap); + } + +} diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java index 5ef974ef6c8..d096a6f2b7d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java +++ b/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 synchronized void setAMRMToken(Token amRMToken) { @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(); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestTimelineServiceHelper.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestTimelineServiceHelper.java new file mode 100644 index 00000000000..f852df0a198 --- /dev/null +++ b/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 nullMap = null; + Assert.assertNull(TimelineServiceHelper.mapCastToHashMap(nullMap)); + + // Test empty hashmap be casted to a empty hashmap + Map emptyHashMap = new HashMap(); + Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(emptyHashMap).size(), 0); + + // Test empty non-hashmap be casted to a empty hashmap + Map emptyTreeMap = new TreeMap(); + Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(emptyTreeMap).size(), 0); + + // Test non-empty hashmap be casted to hashmap correctly + Map firstHashMap = new HashMap(); + 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 firstTreeMap = new TreeMap(); + firstTreeMap.put(key, value); + HashMap alternateHashMap = + TimelineServiceHelper.mapCastToHashMap(firstTreeMap); + Assert.assertEquals(firstTreeMap.size(), alternateHashMap.size()); + Assert.assertEquals(alternateHashMap.get(key), value); + + // Test complicated hashmap be casted correctly + Map> complicatedHashMap = new HashMap>(); + Set hashSet = new HashSet(); + hashSet.add(value); + complicatedHashMap.put(key, hashSet); + Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(complicatedHashMap), + complicatedHashMap); + + // Test complicated non-hashmap get casted correctly + Map> complicatedTreeMap = new TreeMap>(); + complicatedTreeMap.put(key, hashSet); + Assert.assertEquals(TimelineServiceHelper.mapCastToHashMap(complicatedTreeMap).get(key), + hashSet); + } + +}