YARN-3276. Code cleanup for timeline service API records. Contributed by Junping Du.
(cherry picked from commit d88f30ba5359f59fb71b93a55e1c1d9a1c0dff8e) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e4a68dfe Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e4a68dfe Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e4a68dfe Branch: refs/heads/YARN-2928-rebase Commit: e4a68dfe6180321fc71c59e1a56368f4a3881d8c Parents: f5b2f22 Author: Zhijie Shen <zjs...@apache.org> Authored: Wed Jun 3 15:13:29 2015 -0700 Committer: Sangjin Lee <sj...@apache.org> Committed: Mon Nov 9 16:13:07 2015 -0800 ---------------------------------------------------------------------- hadoop-yarn-project/CHANGES.txt | 3 + .../api/records/timeline/TimelineEntity.java | 21 ++---- .../api/records/timeline/TimelineEvent.java | 8 +-- .../records/timelineservice/TimelineEntity.java | 33 ++------- .../records/timelineservice/TimelineEvent.java | 7 +- .../records/timelineservice/TimelineMetric.java | 2 +- .../hadoop/yarn/util/TimelineServiceHelper.java | 47 ++++++++++++ .../impl/pb/AllocateResponsePBImpl.java | 4 +- .../yarn/util/TestTimelineServiceHelper.java | 76 ++++++++++++++++++++ 9 files changed, 147 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index a09058a..406cc51 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/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 http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEntity.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 a43259b..e695050 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 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); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timeline/TimelineEvent.java ---------------------------------------------------------------------- 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 73b2e72..d5611f8 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 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 http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java ---------------------------------------------------------------------- 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 3be7f52..defadec 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 @@ 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); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java ---------------------------------------------------------------------- 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 517c88c..b6b82a7 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 @@ 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) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java ---------------------------------------------------------------------- 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 c897d39..172b56c 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 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); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/TimelineServiceHelper.java ---------------------------------------------------------------------- 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 0000000..ff6ebbd --- /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 <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); + } + +} http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java ---------------------------------------------------------------------- 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 5536e80..7176146 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 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(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e4a68dfe/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-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 0000000..f852df0 --- /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<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); + } + +}