Skip to content

Commit

Permalink
concurrent configuration with test updated
Browse files Browse the repository at this point in the history
  • Loading branch information
xhad1234 committed Aug 1, 2016
1 parent 47a0a40 commit f4dc47a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 50 deletions.
14 changes: 6 additions & 8 deletions core/client/src/test/java/alluxio/hadoop/ConfUtilsTest.java
Expand Up @@ -17,8 +17,6 @@
import org.junit.Assert;
import org.junit.Test;

import java.util.Map;

/**
* Tests for the {@link ConfUtils} class.
*/
Expand All @@ -36,10 +34,10 @@ public void mergeEmptyHadoopConfigurationTest() {
Configuration.defaultInit();
org.apache.hadoop.conf.Configuration hadoopConfig = new org.apache.hadoop.conf.Configuration();

Map<String, String> before = Configuration.toMap();
long beforeSize = Configuration.toMap().size();
ConfUtils.mergeHadoopConfiguration(hadoopConfig);
Map<String, String> after = Configuration.toMap();
Assert.assertEquals(before.size(), after.size());
long afterSize = Configuration.toMap().size();
Assert.assertEquals(beforeSize, afterSize);
}

/**
Expand All @@ -56,10 +54,10 @@ public void mergeHadoopConfigurationTest() {
// This hadoop config will not be loaded into Alluxio configuration.
hadoopConfig.set("hadoop.config.parameter", "hadoop config value");

Map<String, String> before = Configuration.toMap();
long beforeSize = Configuration.toMap().size();
ConfUtils.mergeHadoopConfiguration(hadoopConfig);
Map<String, String> after = Configuration.toMap();
Assert.assertEquals(before.size() + 3, after.size());
long afterSize = Configuration.toMap().size();
Assert.assertEquals(beforeSize + 3, afterSize);
Assert.assertEquals(TEST_S3_ACCCES_KEY, Configuration.get(Constants.S3N_ACCESS_KEY));
Assert.assertEquals(TEST_S3_SECRET_KEY, Configuration.get(Constants.S3N_SECRET_KEY));
Assert.assertEquals(TEST_ALLUXIO_VALUE, Configuration.get(TEST_ALLUXIO_PROPERTY));
Expand Down
87 changes: 48 additions & 39 deletions core/common/src/main/java/alluxio/Configuration.java
Expand Up @@ -20,12 +20,12 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import io.netty.util.internal.chmv8.ConcurrentHashMapV8;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -69,8 +69,9 @@ public final class Configuration {
private static final String REGEX_STRING = "(\\$\\{([^{}]*)\\})";
/** Regex to find ${key} for variable substitution. */
private static final Pattern CONF_REGEX = Pattern.compile(REGEX_STRING);
/** Set of properties. */
private static final Properties PROPERTIES = new Properties();
/** Map of properties. */
private static final ConcurrentHashMapV8<String, String> PROPERTIES_MAP =
new ConcurrentHashMapV8<>();

static {
defaultInit();
Expand Down Expand Up @@ -131,18 +132,18 @@ private static void init(String sitePropertiesFile, boolean includeSystemPropert
}

// Now lets combine, order matters here
PROPERTIES.putAll(defaultProps);
putFromProperties(PROPERTIES_MAP, defaultProps);
if (siteProps != null) {
PROPERTIES.putAll(siteProps);
putFromProperties(PROPERTIES_MAP, siteProps);
}
PROPERTIES.putAll(systemProps);
putFromProperties(PROPERTIES_MAP, systemProps);

String masterHostname = PROPERTIES.getProperty(Constants.MASTER_HOSTNAME);
String masterPort = PROPERTIES.getProperty(Constants.MASTER_RPC_PORT);
boolean useZk = Boolean.parseBoolean(PROPERTIES.getProperty(Constants.ZOOKEEPER_ENABLED));
String masterHostname = PROPERTIES_MAP.get(Constants.MASTER_HOSTNAME);
String masterPort = PROPERTIES_MAP.get(Constants.MASTER_RPC_PORT);
boolean useZk = Boolean.parseBoolean(PROPERTIES_MAP.get(Constants.ZOOKEEPER_ENABLED));
String masterAddress =
(useZk ? Constants.HEADER_FT : Constants.HEADER) + masterHostname + ":" + masterPort;
PROPERTIES.setProperty(Constants.MASTER_ADDRESS, masterAddress);
PROPERTIES_MAP.put(Constants.MASTER_ADDRESS, masterAddress);
checkUserFileBufferBytes();

// Make sure the user hasn't set worker ports when there may be multiple workers per host
Expand All @@ -156,9 +157,15 @@ private static void init(String sitePropertiesFile, boolean includeSystemPropert
String.format(message, Constants.WORKER_RPC_PORT));
Preconditions.checkState(System.getProperty(Constants.WORKER_WEB_PORT) == null,
String.format(message, Constants.WORKER_WEB_PORT));
PROPERTIES.setProperty(Constants.WORKER_DATA_PORT, "0");
PROPERTIES.setProperty(Constants.WORKER_RPC_PORT, "0");
PROPERTIES.setProperty(Constants.WORKER_WEB_PORT, "0");
PROPERTIES_MAP.put(Constants.WORKER_DATA_PORT, "0");
PROPERTIES_MAP.put(Constants.WORKER_RPC_PORT, "0");
PROPERTIES_MAP.put(Constants.WORKER_WEB_PORT, "0");
}
}

private static void putFromProperties(Map<String, String> map, Properties properties) {
for (String key : properties.stringPropertyNames()) {
map.put(key, properties.getProperty(key));
}
}

Expand All @@ -171,7 +178,9 @@ private static void init(String sitePropertiesFile, boolean includeSystemPropert
public static void merge(Map<?, ?> properties) {
if (properties != null) {
// merge the system properties
PROPERTIES.putAll(properties);
for (Map.Entry<?, ?> entry : properties.entrySet()) {
PROPERTIES_MAP.put(entry.getKey().toString(), entry.getValue().toString());
}
}
checkUserFileBufferBytes();
}
Expand All @@ -189,7 +198,7 @@ public static void merge(Map<?, ?> properties) {
public static void set(String key, String value) {
Preconditions.checkArgument(key != null && value != null,
String.format("the key value pair (%s, %s) cannot have null", key, value));
PROPERTIES.put(key, value);
PROPERTIES_MAP.put(key, value);
checkUserFileBufferBytes();
}

Expand All @@ -200,11 +209,11 @@ public static void set(String key, String value) {
* @return the value for the given key
*/
public static String get(String key) {
if (!PROPERTIES.containsKey(key)) {
if (!PROPERTIES_MAP.containsKey(key)) {
// if key is not found among the default properties
throw new RuntimeException(ExceptionMessage.INVALID_CONFIGURATION_KEY.getMessage(key));
}
String raw = PROPERTIES.getProperty(key);
String raw = PROPERTIES_MAP.get(key);
return lookup(raw);
}

Expand All @@ -215,7 +224,7 @@ public static String get(String key) {
* @return true if the key is in the {@link Properties}, false otherwise
*/
public static boolean containsKey(String key) {
return PROPERTIES.containsKey(key);
return PROPERTIES_MAP.containsKey(key);
}

/**
Expand All @@ -225,8 +234,8 @@ public static boolean containsKey(String key) {
* @return the value for the given key as an {@code int}
*/
public static int getInt(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
try {
return Integer.parseInt(lookup(rawValue));
} catch (NumberFormatException e) {
Expand All @@ -244,8 +253,8 @@ public static int getInt(String key) {
* @return the value for the given key as a {@code long}
*/
public static long getLong(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
try {
return Long.parseLong(lookup(rawValue));
} catch (NumberFormatException e) {
Expand All @@ -263,8 +272,8 @@ public static long getLong(String key) {
* @return the value for the given key as a {@code double}
*/
public static double getDouble(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
try {
return Double.parseDouble(lookup(rawValue));
} catch (NumberFormatException e) {
Expand All @@ -282,8 +291,8 @@ public static double getDouble(String key) {
* @return the value for the given key as a {@code float}
*/
public static float getFloat(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
try {
return Float.parseFloat(lookup(rawValue));
} catch (NumberFormatException e) {
Expand All @@ -301,8 +310,8 @@ public static float getFloat(String key) {
* @return the value for the given key as a {@code boolean}
*/
public static boolean getBoolean(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
String value = lookup(rawValue);
if (value.equalsIgnoreCase("true")) {
return true;
Expand All @@ -326,8 +335,8 @@ public static boolean getBoolean(String key) {
public static List<String> getList(String key, String delimiter) {
Preconditions.checkArgument(delimiter != null, "Illegal separator for Alluxio properties as "
+ "list");
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
return Lists.newLinkedList(Splitter.on(delimiter).trimResults().omitEmptyStrings()
.split(rawValue));
}
Expand All @@ -344,7 +353,7 @@ public static List<String> getList(String key, String delimiter) {
* @return the value for the given key as an enum value
*/
public static <T extends Enum<T>> T getEnum(String key, Class<T> enumType) {
if (!PROPERTIES.containsKey(key)) {
if (!PROPERTIES_MAP.containsKey(key)) {
throw new RuntimeException(ExceptionMessage.INVALID_CONFIGURATION_KEY.getMessage(key));
}
final String val = get(key);
Expand All @@ -358,7 +367,7 @@ public static <T extends Enum<T>> T getEnum(String key, Class<T> enumType) {
* @return the bytes of the value for the given key
*/
public static long getBytes(String key) {
if (PROPERTIES.containsKey(key)) {
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = get(key);
try {
return FormatUtils.parseSpaceSize(rawValue);
Expand All @@ -377,8 +386,8 @@ public static long getBytes(String key) {
* @return the value for the given key as a class
*/
public static <T> Class<T> getClass(String key) {
if (PROPERTIES.containsKey(key)) {
String rawValue = PROPERTIES.getProperty(key);
if (PROPERTIES_MAP.containsKey(key)) {
String rawValue = PROPERTIES_MAP.get(key);
try {
@SuppressWarnings("unchecked")
Class<T> clazz = (Class<T>) Class.forName(rawValue);
Expand All @@ -393,10 +402,10 @@ public static <T> Class<T> getClass(String key) {
}

/**
* @return a copy of the internal {@link Properties} of as an immutable map
* @return a view of the internal {@link Properties} of as an immutable map
*/
public static ImmutableMap<String, String> toMap() {
return Maps.fromProperties(PROPERTIES);
public static Map<String, String> toMap() {
return Collections.unmodifiableMap(PROPERTIES_MAP);
}

/**
Expand Down Expand Up @@ -430,7 +439,7 @@ private static String lookupRecursively(final String base, Map<String, String> f
String match = matcher.group(2).trim();
String value;
if (!found.containsKey(match)) {
value = lookupRecursively(PROPERTIES.getProperty(match), found);
value = lookupRecursively(PROPERTIES_MAP.get(match), found);
found.put(match, value);
} else {
value = found.get(match);
Expand Down
6 changes: 3 additions & 3 deletions core/common/src/test/java/alluxio/ConfigurationTestUtils.java
Expand Up @@ -11,13 +11,12 @@

package alluxio;

import io.netty.util.internal.chmv8.ConcurrentHashMapV8;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.reflect.Whitebox;

import java.util.Properties;

/**
* Utility methods for the configuration tests.
*/
Expand All @@ -32,7 +31,8 @@ public final class ConfigurationTestUtils {
* while any object may be using the {@link Configuration}.
*/
public static void resetConfiguration() {
Properties properties = Whitebox.getInternalState(Configuration.class, "PROPERTIES");
ConcurrentHashMapV8<String, String>
properties = Whitebox.getInternalState(Configuration.class, "PROPERTIES_MAP");
properties.clear();
Configuration.defaultInit();
}
Expand Down

0 comments on commit f4dc47a

Please sign in to comment.