You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by ch...@apache.org on 2016/12/13 23:35:00 UTC
twill git commit: Fix ConcurrentModificationException from removal in
map and refactor the code
Repository: twill
Updated Branches:
refs/heads/master 6edf214ba -> 13885f126
Fix ConcurrentModificationException from removal in map and refactor the code
- Removing loglevel from the map during iteration will cause ConcurrentModificationException.
- Now Using Iterator to iterate the map and remove. Also refactored the code a bit.
This closes #19 on Github.
Signed-off-by: Terence Yim <ch...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/13885f12
Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/13885f12
Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/13885f12
Branch: refs/heads/master
Commit: 13885f1261634715ffd9cb0a28aba86db02213ce
Parents: 6edf214
Author: yaojiefeng <ya...@cask.co>
Authored: Tue Dec 13 15:08:54 2016 -0800
Committer: Terence Yim <ch...@apache.org>
Committed: Tue Dec 13 15:34:53 2016 -0800
----------------------------------------------------------------------
.../twill/internal/state/SystemMessages.java | 4 +-
.../appmaster/ApplicationMasterService.java | 2 +-
.../internal/appmaster/RunningContainers.java | 2 +-
.../container/TwillContainerService.java | 99 ++++++++++++--------
4 files changed, 66 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/twill/blob/13885f12/twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java
----------------------------------------------------------------------
diff --git a/twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java b/twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java
index ff5888b..1b5363a 100644
--- a/twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java
+++ b/twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java
@@ -33,7 +33,7 @@ import javax.annotation.Nullable;
*/
public final class SystemMessages {
- public static final String LOG_LEVEL = "logLevels";
+ public static final String SET_LOG_LEVEL = "setLogLevels";
public static final String RESET_LOG_LEVEL = "resetLogLevels";
public static final Command STOP_COMMAND = Command.Builder.of("stop").build();
public static final Message SECURE_STORE_UPDATED = new SimpleMessage(
@@ -101,7 +101,7 @@ public final class SystemMessages {
Map<String, String> options = convertLogEntryToString(logLevels);
return new SimpleMessage(Message.Type.SYSTEM,
runnableName == null ? Message.Scope.ALL_RUNNABLE : Message.Scope.RUNNABLE,
- runnableName, Command.Builder.of(LOG_LEVEL).addOptions(options).build());
+ runnableName, Command.Builder.of(SET_LOG_LEVEL).addOptions(options).build());
}
/**
http://git-wip-us.apache.org/repos/asf/twill/blob/13885f12/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
index 9faadc2..c1c3986 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
@@ -970,7 +970,7 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp
}
String command = message.getCommand().getCommand();
- if (!command.equals(SystemMessages.LOG_LEVEL) && !command.equals(SystemMessages.RESET_LOG_LEVEL)) {
+ if (!command.equals(SystemMessages.SET_LOG_LEVEL) && !command.equals(SystemMessages.RESET_LOG_LEVEL)) {
return false;
}
http://git-wip-us.apache.org/repos/asf/twill/blob/13885f12/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
index e6234c3..d0fad26 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
@@ -599,7 +599,7 @@ final class RunningContainers {
private void checkAndUpdateLogLevels(Message message, String runnableName) {
String command = message.getCommand().getCommand();
- if (message.getType() != Message.Type.SYSTEM || (!SystemMessages.LOG_LEVEL.equals(command) &&
+ if (message.getType() != Message.Type.SYSTEM || (!SystemMessages.SET_LOG_LEVEL.equals(command) &&
!SystemMessages.RESET_LOG_LEVEL.equals(command))) {
return;
}
http://git-wip-us.apache.org/repos/asf/twill/blob/13885f12/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java b/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
index fa38115..58298a0 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
@@ -45,7 +45,9 @@ import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import javax.annotation.Nullable;
@@ -113,45 +115,22 @@ public final class TwillContainerService extends AbstractYarnTwillService {
}
String commandStr = command.getCommand();
- if (message.getType() == Message.Type.SYSTEM &&
- (SystemMessages.LOG_LEVEL.equals(commandStr) || SystemMessages.RESET_LOG_LEVEL.equals(commandStr))) {
- if (SystemMessages.RESET_LOG_LEVEL.equals(commandStr)) {
- Map<String, String> loggerNames = command.getOptions();
- // Reset
- for (Map.Entry<String, String> entry : oldLogLevels.entrySet()) {
- String loggerName = entry.getKey();
- // logger name is empty if we are resetting all loggers.
- if (loggerNames.isEmpty() || loggerNames.containsKey(loggerName)) {
- String oldLogLevel = entry.getValue();
-
- setLogLevel(loggerName, oldLogLevel);
- if (oldLogLevel == null || !defaultLogLevels.containsKey(loggerName)) {
- containerLiveNodeData.removeLogLevel(loggerName);
- oldLogLevels.remove(loggerName);
- } else {
- containerLiveNodeData.setLogLevel(loggerName, oldLogLevel);
- }
- }
- }
- } else {
- // Set log levels
- for (Map.Entry<String, String> entry : command.getOptions().entrySet()) {
- String loggerName = entry.getKey();
- String logLevel = entry.getValue();
-
- // Setting the log level in logging system as well as in the container live node
- String oldLogLevel = setLogLevel(loggerName, logLevel);
- containerLiveNodeData.setLogLevel(loggerName, logLevel);
-
- if (!oldLogLevels.containsKey(loggerName)) {
- String defaultLogLevel = defaultLogLevels.get(loggerName);
- oldLogLevels.put(loggerName, defaultLogLevel == null ? oldLogLevel : defaultLogLevel);
- }
- }
+ if (message.getType() == Message.Type.SYSTEM) {
+ boolean handled = false;
+ if (SystemMessages.SET_LOG_LEVEL.equals(commandStr)) {
+ // The options is a map from logger name to log level.
+ setLogLevels(command.getOptions());
+ handled = true;
+ } else if (SystemMessages.RESET_LOG_LEVEL.equals(commandStr)) {
+ // The options is a set of loggers to reset in the form of loggerName -> loggerName map.
+ resetLogLevels(command.getOptions().keySet());
+ handled = true;
}
- updateLiveNode();
- return Futures.immediateFuture(messageId);
+ if (handled) {
+ updateLiveNode();
+ return Futures.immediateFuture(messageId);
+ }
}
commandExecutor.execute(new Runnable() {
@@ -169,6 +148,52 @@ public final class TwillContainerService extends AbstractYarnTwillService {
return result;
}
+ /**
+ * Sets the log levels for the given set of loggers.
+ *
+ * @param logLevels a map from logger name to log level to be set
+ */
+ private void setLogLevels(Map<String, String> logLevels) {
+ for (Map.Entry<String, String> entry : logLevels.entrySet()) {
+ String loggerName = entry.getKey();
+ String logLevel = entry.getValue();
+
+ // Setting the log level in logging system as well as in the container live node
+ String oldLogLevel = setLogLevel(loggerName, logLevel);
+ containerLiveNodeData.setLogLevel(loggerName, logLevel);
+
+ if (!oldLogLevels.containsKey(loggerName)) {
+ String defaultLogLevel = defaultLogLevels.get(loggerName);
+ oldLogLevels.put(loggerName, defaultLogLevel == null ? oldLogLevel : defaultLogLevel);
+ }
+ }
+ }
+
+ /**
+ * Resets the log levels for the given set of loggers. If the set is empty, reset all loggers that have been set
+ * before.
+ */
+ private void resetLogLevels(Set<String> loggerNames) {
+ Iterator<Map.Entry<String, String>> entryIterator = oldLogLevels.entrySet().iterator();
+ while (entryIterator.hasNext()) {
+ Map.Entry<String, String> entry = entryIterator.next();
+
+ String loggerName = entry.getKey();
+ // logger name is empty if we are resetting all loggers.
+ if (loggerNames.isEmpty() || loggerNames.contains(loggerName)) {
+ String oldLogLevel = entry.getValue();
+
+ setLogLevel(loggerName, oldLogLevel);
+ if (oldLogLevel == null || !defaultLogLevels.containsKey(loggerName)) {
+ containerLiveNodeData.removeLogLevel(loggerName);
+ entryIterator.remove();
+ } else {
+ containerLiveNodeData.setLogLevel(loggerName, oldLogLevel);
+ }
+ }
+ }
+ }
+
@SuppressWarnings("unchecked")
@Override
protected void doStart() throws Exception {