brumi1024 commented on code in PR #8801: URL: https://github.com/apache/hadoop/pull/8801#discussion_r4243510541
########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/resolver/ConfigSnapshot.java: ########## @@ -0,0 +1,304 @@ +/** + * 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.server.resourcemanager.scheduler.capacity.resolver; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.conf.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * An immutable copy of a {@link Configuration}, taken once, with a prefix + * index for per-queue and per-label lookups. + * <p> + * {@link #get(String)} and {@link #getPropertiesWithPrefix(String)} return + * the value {@code conf.get(key)} returned when the snapshot was taken, so + * deprecation handling and variable substitution are applied. The raw + * accessors return the unexpanded text stored in the configuration, which is + * what the prefix lookups of the Capacity Scheduler have always used. + * <p> + * The prefix index is a trie with one node per key part delimited by ".". + * A key is stored in the node of its last part, so a prefix query walks the + * prefix parts and collects the whole subtree below the last one. Prefixes + * therefore match whole key parts only: {@code root.a} matches + * {@code root.a.capacity} but not {@code root.ab.capacity}. + */ [email protected] [email protected] +public final class ConfigSnapshot { + private static final Logger LOG = + LoggerFactory.getLogger(ConfigSnapshot.class); + + private static final String DELIMITER = "\\."; + private static final String DOT = "."; + + /** Key to the value {@code conf.get(key)} returned. */ + private final Map<String, String> values; + /** + * Key to the raw text, only for keys whose raw text differs from the + * value, which keeps the common case free of a second copy. + */ + private final Map<String, String> rawValues; + /** Keys whose {@code conf.get(key)} threw, rethrown on access. */ + private final Map<String, RuntimeException> failures; + private final Set<String> keys; + private final PrefixNode root = new PrefixNode(); + + private ConfigSnapshot(Map<String, String> values, + Map<String, String> rawValues, Map<String, RuntimeException> failures, + Set<String> keys) { + this.values = values; + this.rawValues = rawValues; + this.failures = failures; + this.keys = Collections.unmodifiableSet(keys); + for (String key : keys) { + index(key); + } + } + + /** + * Takes a snapshot of every property of a configuration. + * @param conf the configuration to copy + * @return the snapshot + */ + public static ConfigSnapshot of(Configuration conf) { + Map<String, String> raw = new HashMap<>(); + // The iterator returns a copy of the raw properties, so the conf.get + // calls below, which may update deprecated keys, do not disturb it. + Iterator<Map.Entry<String, String>> it = conf.iterator(); + while (it.hasNext()) { + Map.Entry<String, String> entry = it.next(); + raw.put(entry.getKey(), entry.getValue()); + } + + Map<String, String> values = new HashMap<>(raw.size() * 4 / 3 + 1); + Map<String, String> rawValues = new HashMap<>(); + Map<String, RuntimeException> failures = new HashMap<>(); + for (Map.Entry<String, String> entry : raw.entrySet()) { + String key = entry.getKey(); + String rawValue = entry.getValue(); + // conf.get only differs from the raw text when the value references a + // variable or the key is deprecated; skipping the call keeps the + // snapshot cheap for the common case. + if (rawValue.indexOf('$') < 0 && !Configuration.isDeprecated(key)) { + values.put(key, rawValue); + continue; + } + try { + String value = conf.get(key); + if (value != null) { + values.put(key, value); + } + if (!rawValue.equals(value)) { + rawValues.put(key, rawValue); + } + } catch (RuntimeException e) { + failures.put(key, e); + rawValues.put(key, rawValue); + } + } + return new ConfigSnapshot(values, rawValues, failures, raw.keySet()); + } + + /** + * Takes a snapshot of plain properties, using every value as it is, without + * deprecation handling or variable substitution. + * @param properties the properties to copy + * @return the snapshot + */ + public static ConfigSnapshot of(Map<String, String> properties) { + return new ConfigSnapshot(new HashMap<>(properties), + Collections.<String, String>emptyMap(), + Collections.<String, RuntimeException>emptyMap(), + new HashSet<>(properties.keySet())); + } + + /** + * Returns the value of a property. + * @param key the full property key + * @return the value {@code conf.get(key)} returned, or {@code null} when + * the property is absent + * @throws RuntimeException the exception {@code conf.get(key)} threw, for + * example on a too deep variable substitution + */ + public String get(String key) { Review Comment: The expanded accessors are used by QueueConfigResolver in YARN-12015; I kept them here so the snapshot's semantics are reviewed in one place. The extra cost is one conf.get() per value containing $ or per deprecated key. I'll add the benchmark numbers to the PR description. ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/resolver/ConfigSnapshot.java: ########## @@ -0,0 +1,304 @@ +/** + * 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.server.resourcemanager.scheduler.capacity.resolver; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.conf.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * An immutable copy of a {@link Configuration}, taken once, with a prefix + * index for per-queue and per-label lookups. + * <p> + * {@link #get(String)} and {@link #getPropertiesWithPrefix(String)} return + * the value {@code conf.get(key)} returned when the snapshot was taken, so + * deprecation handling and variable substitution are applied. The raw + * accessors return the unexpanded text stored in the configuration, which is + * what the prefix lookups of the Capacity Scheduler have always used. + * <p> + * The prefix index is a trie with one node per key part delimited by ".". + * A key is stored in the node of its last part, so a prefix query walks the + * prefix parts and collects the whole subtree below the last one. Prefixes + * therefore match whole key parts only: {@code root.a} matches + * {@code root.a.capacity} but not {@code root.ab.capacity}. + */ [email protected] [email protected] +public final class ConfigSnapshot { + private static final Logger LOG = + LoggerFactory.getLogger(ConfigSnapshot.class); + + private static final String DELIMITER = "\\."; + private static final String DOT = "."; + + /** Key to the value {@code conf.get(key)} returned. */ + private final Map<String, String> values; + /** + * Key to the raw text, only for keys whose raw text differs from the + * value, which keeps the common case free of a second copy. + */ + private final Map<String, String> rawValues; + /** Keys whose {@code conf.get(key)} threw, rethrown on access. */ + private final Map<String, RuntimeException> failures; + private final Set<String> keys; + private final PrefixNode root = new PrefixNode(); + + private ConfigSnapshot(Map<String, String> values, + Map<String, String> rawValues, Map<String, RuntimeException> failures, + Set<String> keys) { + this.values = values; + this.rawValues = rawValues; + this.failures = failures; + this.keys = Collections.unmodifiableSet(keys); + for (String key : keys) { + index(key); + } + } + + /** + * Takes a snapshot of every property of a configuration. + * @param conf the configuration to copy + * @return the snapshot + */ + public static ConfigSnapshot of(Configuration conf) { + Map<String, String> raw = new HashMap<>(); + // The iterator returns a copy of the raw properties, so the conf.get + // calls below, which may update deprecated keys, do not disturb it. + Iterator<Map.Entry<String, String>> it = conf.iterator(); + while (it.hasNext()) { + Map.Entry<String, String> entry = it.next(); + raw.put(entry.getKey(), entry.getValue()); + } + + Map<String, String> values = new HashMap<>(raw.size() * 4 / 3 + 1); + Map<String, String> rawValues = new HashMap<>(); + Map<String, RuntimeException> failures = new HashMap<>(); + for (Map.Entry<String, String> entry : raw.entrySet()) { + String key = entry.getKey(); + String rawValue = entry.getValue(); + // conf.get only differs from the raw text when the value references a + // variable or the key is deprecated; skipping the call keeps the + // snapshot cheap for the common case. + if (rawValue.indexOf('$') < 0 && !Configuration.isDeprecated(key)) { + values.put(key, rawValue); + continue; + } + try { + String value = conf.get(key); + if (value != null) { + values.put(key, value); + } + if (!rawValue.equals(value)) { + rawValues.put(key, rawValue); + } + } catch (RuntimeException e) { + failures.put(key, e); + rawValues.put(key, rawValue); + } + } + return new ConfigSnapshot(values, rawValues, failures, raw.keySet()); + } + + /** + * Takes a snapshot of plain properties, using every value as it is, without + * deprecation handling or variable substitution. + * @param properties the properties to copy + * @return the snapshot + */ + public static ConfigSnapshot of(Map<String, String> properties) { + return new ConfigSnapshot(new HashMap<>(properties), + Collections.<String, String>emptyMap(), + Collections.<String, RuntimeException>emptyMap(), + new HashSet<>(properties.keySet())); + } + + /** + * Returns the value of a property. + * @param key the full property key + * @return the value {@code conf.get(key)} returned, or {@code null} when + * the property is absent + * @throws RuntimeException the exception {@code conf.get(key)} threw, for + * example on a too deep variable substitution + */ + public String get(String key) { + RuntimeException failure = failures.get(key); + if (failure != null) { + throw failure; Review Comment: Fixed. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
