[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15744792#comment-15744792 ] ASF GitHub Bot commented on TWILL-138: -- Github user yaojiefeng commented on the issue: https://github.com/apache/twill/pull/14 Thanks! @hsaputra > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra >Assignee: Yaojie Feng > Fix For: 0.9.0 > > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15743241#comment-15743241 ] ASF GitHub Bot commented on TWILL-138: -- Github user hsaputra commented on the issue: https://github.com/apache/twill/pull/14 Congrats @yaojiefeng ! =) > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra >Assignee: Yaojie Feng > Fix For: 0.9.0 > > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15740552#comment-15740552 ] ASF GitHub Bot commented on TWILL-138: -- Github user asfgit closed the pull request at: https://github.com/apache/twill/pull/14 > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15740545#comment-15740545 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on the issue: https://github.com/apache/twill/pull/14 LGTM > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738859#comment-15738859
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91846095
--- Diff:
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---
@@ -384,6 +408,24 @@ private void setEnv(String runnableName, Map env, boolean overwr
}
}
+ private void saveLogLevels(LogEntry.Level level) {
+level = level == null ? LogEntry.Level.INFO : level;
+Map appLogLevels = new HashMap<>();
+appLogLevels.put(Logger.ROOT_LOGGER_NAME, level.name());
+for (String runnableName : twillSpec.getRunnables().keySet()) {
+ this.logLevels.put(runnableName, appLogLevels);
+}
+ }
+
+ private void saveLogLevels(String runnableName, Map logLevels) {
+Map newLevels = new HashMap<>();
+for (Map.Entry entry : logLevels.entrySet()) {
+ Preconditions.checkArgument(entry.getValue() != null, "Log level
cannot be null for logger {}", entry.getKey());
+ newLevels.put(entry.getKey(), entry.getValue().name());
+}
+this.logLevels.get(runnableName).putAll(newLevels);
--- End diff --
What I meant was, since the method is "set", it should be setting /
overwriting, not appending. And I think it's ok for the user to gather and pass
in the map.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738781#comment-15738781
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91845583
--- Diff:
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---
@@ -384,6 +408,24 @@ private void setEnv(String runnableName, Map env, boolean overwr
}
}
+ private void saveLogLevels(LogEntry.Level level) {
+level = level == null ? LogEntry.Level.INFO : level;
+Map appLogLevels = new HashMap<>();
+appLogLevels.put(Logger.ROOT_LOGGER_NAME, level.name());
+for (String runnableName : twillSpec.getRunnables().keySet()) {
+ this.logLevels.put(runnableName, appLogLevels);
+}
+ }
+
+ private void saveLogLevels(String runnableName, Map logLevels) {
+Map newLevels = new HashMap<>();
+for (Map.Entry entry : logLevels.entrySet()) {
+ Preconditions.checkArgument(entry.getValue() != null, "Log level
cannot be null for logger {}", entry.getKey());
+ newLevels.put(entry.getKey(), entry.getValue().name());
+}
+this.logLevels.get(runnableName).putAll(newLevels);
--- End diff --
Will this make the `setLogLevels` complicated for the user? For example, if
user wants to change several log levels for the entire application. He also
wants to set a particular logger for the runnable. He will have to have to pass
in the entire map to `setLogLevels(runnableName, logLevelsMap)`. Is it good to
do so?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738740#comment-15738740
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91845224
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevels();
--- End diff --
Changed the value in `Future` to be the loggerNames passed through this
methods. Empty means resetting for all loggers
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738709#comment-15738709
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844825
--- Diff:
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---
@@ -384,6 +408,24 @@ private void setEnv(String runnableName, Map env, boolean overwr
}
}
+ private void saveLogLevels(LogEntry.Level level) {
+level = level == null ? LogEntry.Level.INFO : level;
--- End diff --
Shouldn't default it to `INFO` at all. It should be treated the same as
`null` is provided as the log level value as in the other `saveLogLevels`
method, which will throw `IllegalArgumentException`. Also, rename the `level`
parameter to `rootLogLevel` to be clear.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738708#comment-15738708
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844732
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,48 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables.
+ * The log levels will be the same as when the runnables start up.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. The future result
+ * is the logger names provided in the parameter.
+ */
+ Future resetLogLevels(String...loggerNames);
+
+ /**
+ * Reset the log levels of the given runnable.
+ * The log levels will be same as when the runnable starts up.
+ *
--- End diff --
Missed the `@param loggerNames` doc.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738707#comment-15738707
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844862
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillPreparer.java
---
@@ -227,15 +227,36 @@
TwillPreparer addSecureStore(SecureStore secureStore);
/**
- * Set the log level for Twill applications running in a container.
+ * Set the root log level for Twill applications in all containers.
*
* @param logLevel the {@link LogEntry.Level} that should be set.
* The level match the {@code Logback} levels.
* @return This {@link TwillPreparer}.
+ * @deprecated Use {@link #setLogLevels(Map)} with key {@link
org.slf4j.Logger#ROOT_LOGGER_NAME} instead.
*/
+ @Deprecated
TwillPreparer setLogLevel(LogEntry.Level logLevel);
/**
+ * Set the log levels for requested logger names for Twill applications
running in a container. The log level whose
+ * value is null will be ignored
--- End diff --
I think the implementation actually throws `IllegalArgumentException`,
which is better than silently ignored. Please update the doc here accordingly.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738706#comment-15738706 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r91844834 --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java --- @@ -151,8 +152,9 @@ this.reservedMemory = yarnConfig.getInt(Configs.Keys.JAVA_RESERVED_MEMORY_MB, Configs.Defaults.JAVA_RESERVED_MEMORY_MB); this.extraOptions = extraOptions; -this.logLevel = logLevel; this.classAcceptor = new ClassAcceptor(); +this.logLevel = logLevel; --- End diff -- Shouldn't have this field anymore. Every log level info should be in the `logLevels` map > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738713#comment-15738713
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844729
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,48 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables.
+ * The log levels will be the same as when the runnables start up.
+ *
--- End diff --
Missed the `@param loggerNames` doc.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738704#comment-15738704
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90134323
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
--- End diff --
Is the default always `INFO`? It's better not to say that.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738710#comment-15738710
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90134835
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java ---
@@ -197,16 +198,16 @@ protected final void shutDown() throws Exception {
}
}
+ protected final OperationFuture updateLiveNode() {
--- End diff --
Add a javadoc for protected method as it is meant to be called by the child
class.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738712#comment-15738712
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844710
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
--- End diff --
@hsaputra I think it get removed after this comment, but this comment
sticks.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738711#comment-15738711
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90134468
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevels();
--- End diff --
What's the `String` inside the `Future` represents? If there is nothing
actually returned from the `Future.get()`, you can specify the return type as
`Future` and returns `null` when `Future.get()` is called.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738705#comment-15738705
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r91844855
--- Diff:
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---
@@ -384,6 +408,24 @@ private void setEnv(String runnableName, Map env, boolean overwr
}
}
+ private void saveLogLevels(LogEntry.Level level) {
+level = level == null ? LogEntry.Level.INFO : level;
+Map appLogLevels = new HashMap<>();
+appLogLevels.put(Logger.ROOT_LOGGER_NAME, level.name());
+for (String runnableName : twillSpec.getRunnables().keySet()) {
+ this.logLevels.put(runnableName, appLogLevels);
+}
+ }
+
+ private void saveLogLevels(String runnableName, Map logLevels) {
+Map newLevels = new HashMap<>();
+for (Map.Entry entry : logLevels.entrySet()) {
+ Preconditions.checkArgument(entry.getValue() != null, "Log level
cannot be null for logger {}", entry.getKey());
+ newLevels.put(entry.getKey(), entry.getValue().name());
+}
+this.logLevels.get(runnableName).putAll(newLevels);
--- End diff --
We should always create a new map instead of calling `putAll`. The reason
is the exposed method is call `setLogLevels`, not `addLogLevels` or
`appendLogLevels`, which implies a overwriting instead of appending.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15719105#comment-15719105 ] ASF GitHub Bot commented on TWILL-138: -- Github user anew commented on the issue: https://github.com/apache/twill/pull/14 OK, my concerns are addressed and it LGTM. Please wait for @chtyim to do another pass before this gets merged. > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15719101#comment-15719101
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90768827
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -51,29 +63,36 @@
private final ClassLoader classLoader;
private final BasicTwillContext context;
private final ContainerLiveNodeData containerLiveNodeData;
+ private final Map oldLogLevels;
+ private final Map defaultLogLevels;
private ExecutorService commandExecutor;
private TwillRunnable runnable;
public TwillContainerService(BasicTwillContext context, ContainerInfo
containerInfo, ZKClient zkClient,
RunId runId, TwillRunnableSpecification
specification, ClassLoader classLoader,
- Location applicationLocation) {
+ Location applicationLocation, Map defaultLogLevels,
+ Map logLevels) {
super(zkClient, runId, applicationLocation);
this.specification = specification;
this.classLoader = classLoader;
-this.containerLiveNodeData = createLiveNodeData(containerInfo);
+this.defaultLogLevels = ImmutableMap.copyOf(defaultLogLevels);
+this.oldLogLevels = Collections.synchronizedMap(new
HashMap<>(defaultLogLevels));
--- End diff --
I see. Seems ok.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718955#comment-15718955
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767592
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -51,29 +63,36 @@
private final ClassLoader classLoader;
private final BasicTwillContext context;
private final ContainerLiveNodeData containerLiveNodeData;
+ private final Map oldLogLevels;
+ private final Map defaultLogLevels;
private ExecutorService commandExecutor;
private TwillRunnable runnable;
public TwillContainerService(BasicTwillContext context, ContainerInfo
containerInfo, ZKClient zkClient,
RunId runId, TwillRunnableSpecification
specification, ClassLoader classLoader,
- Location applicationLocation) {
+ Location applicationLocation, Map defaultLogLevels,
+ Map logLevels) {
super(zkClient, runId, applicationLocation);
this.specification = specification;
this.classLoader = classLoader;
-this.containerLiveNodeData = createLiveNodeData(containerInfo);
+this.defaultLogLevels = ImmutableMap.copyOf(defaultLogLevels);
+this.oldLogLevels = Collections.synchronizedMap(new
HashMap<>(defaultLogLevels));
--- End diff --
When you reset, you cannot reset the log level for a particular logger. It
will reset the log levels for all loggers which are changed during runtime. So
if you call reset, it will reset the log level of ROOT to INFO, and the
particular logger to the log level when the runnable starts. If it was null
initially, its log level will be following the ROOT log level, INFO
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718950#comment-15718950
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767535
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -51,29 +63,36 @@
private final ClassLoader classLoader;
private final BasicTwillContext context;
private final ContainerLiveNodeData containerLiveNodeData;
+ private final Map oldLogLevels;
+ private final Map defaultLogLevels;
private ExecutorService commandExecutor;
private TwillRunnable runnable;
public TwillContainerService(BasicTwillContext context, ContainerInfo
containerInfo, ZKClient zkClient,
RunId runId, TwillRunnableSpecification
specification, ClassLoader classLoader,
- Location applicationLocation) {
+ Location applicationLocation, Map defaultLogLevels,
+ Map logLevels) {
super(zkClient, runId, applicationLocation);
this.specification = specification;
this.classLoader = classLoader;
-this.containerLiveNodeData = createLiveNodeData(containerInfo);
+this.defaultLogLevels = ImmutableMap.copyOf(defaultLogLevels);
+this.oldLogLevels = Collections.synchronizedMap(new
HashMap<>(defaultLogLevels));
--- End diff --
The `defaultLogLevels` are the log levels passed with `TwillPreparer`, when
resetting the log levels, we will need it to update the live node info to
indicate the log levels. The `oldLogLevels` is the same as the
`defaultLogLevels` when the runnable starts up. We will not update it for each
log level change. It is used to save old log levels for loggers for the first
time change as we do not know what is in the `logback.xml`. When reset, we
reset based on this map.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718936#comment-15718936
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767420
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevels();
+
+ /**
+ * Reset the log levels of the given runnable to the log levels when .
--- End diff --
My bad for not updating the comments. The `resetLogLevels` will reset the
log level of a runnable to the time when it starts up. This means it will reset
the log level to what `logback.xml` is + the log level requested from
`TwillPreparer`
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718934#comment-15718934
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767395
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevels();
+
+ /**
+ * Reset the log levels of the given runnable to the log levels when .
--- End diff --
Yes. The future is completed when the message is processed. We are not sure
about whether the request is successful or not. Now the live node will be
updated only if the log level set is completed.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718923#comment-15718923
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767209
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,45 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
+ * the specified logger.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevels();
+
+ /**
+ * Reset the log levels of the given runnable to the log levels when .
--- End diff --
this sentence is incomplete. Does this mean restore the log level to what
it was before the last setLogLevel()? Or before setLogLevel was called for the
first time? Or to what it was in the original spec?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718925#comment-15718925
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767079
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
--- End diff --
Still, the way I understand this is that it means, the message was received
by the app master and passed on to the runnables. We have no idea whether the
runnables have received or processed it. That can only be determined by
examining the live node info for each runnable.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718924#comment-15718924
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r90767247
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -51,29 +63,36 @@
private final ClassLoader classLoader;
private final BasicTwillContext context;
private final ContainerLiveNodeData containerLiveNodeData;
+ private final Map oldLogLevels;
+ private final Map defaultLogLevels;
private ExecutorService commandExecutor;
private TwillRunnable runnable;
public TwillContainerService(BasicTwillContext context, ContainerInfo
containerInfo, ZKClient zkClient,
RunId runId, TwillRunnableSpecification
specification, ClassLoader classLoader,
- Location applicationLocation) {
+ Location applicationLocation, Map defaultLogLevels,
+ Map logLevels) {
super(zkClient, runId, applicationLocation);
this.specification = specification;
this.classLoader = classLoader;
-this.containerLiveNodeData = createLiveNodeData(containerInfo);
+this.defaultLogLevels = ImmutableMap.copyOf(defaultLogLevels);
+this.oldLogLevels = Collections.synchronizedMap(new
HashMap<>(defaultLogLevels));
--- End diff --
can you explain what oldLogLevels is for? My understanding is this:
- when a log level is updated, you save previous level in oldLogLevels
- when the log level is reset, you restore the log level from oldLogLevels
Is that correct?
If so, then consider this scenario:
- root log level is INFO
- I debug a problem, so I set the root log level to DEBUG
- I set the log level for one logger to TRACE
- now I found the problem.
- I reset the log level for that particular logger
- then I set the root log level back to INFO
what will be the log level for that particular logger after all this? INFO
or DEBUG?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15700921#comment-15700921
]
ASF GitHub Bot commented on TWILL-138:
--
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r89718330
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
--- End diff --
where did you see the `setRootLogLevel` method definition?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664846#comment-15664846
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87879825
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -90,8 +104,14 @@ public Integer getDebugPort() {
}
@Override
+ @Deprecated
public Level getLogLevel() {
-return logLevel;
+return getLogLevels().get(Logger.ROOT_LOGGER_NAME);
--- End diff --
This will return `null` if the `ContainerLiveNodeData` not getting created
or updated. We want it to be like that since it will reflect the actual state
of the container, right?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664812#comment-15664812
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87877029
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,30 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
--- End diff --
We check it using `Preconditions.checkArgument`, so an exception will be
thrown for null log level for root logger.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664787#comment-15664787
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87874799
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -575,18 +597,63 @@ private boolean removeContainerInfo(String
containerId) {
return false;
}
+ private void checkAndSetLogLevels(Message message, String runnableName) {
+String command = message.getCommand().getCommand();
+if (message.getType() != Message.Type.SYSTEM ||
!SystemMessages.LOG_LEVEL.equals(command)) {
+ return;
+}
+
+Map runnableLogLevels =
copyAndConvertLogLevelsMap(message.getCommand().getOptions());
+if (!logLevels.containsKey(runnableName)) {
+ logLevels.put(runnableName, runnableLogLevels);
+} else {
+ logLevels.get(runnableName).putAll(runnableLogLevels);
+}
+ }
+
+ private Location saveLogLevels() {
+LOG.debug("save the log level file");
+try {
+ Gson gson = new GsonBuilder().serializeNulls().create();
+ String jsonStr = gson.toJson(logLevels);
+ String fileName = Hashing.md5().hashString(jsonStr) + "." +
Constants.Files.LOG_LEVELS;
+ Location location = applicationLocation.append(fileName);
+ if (!location.exists()) {
+try (Writer writer = new
OutputStreamWriter(location.getOutputStream(), Charsets.UTF_8)) {
+ gson.toJson(logLevels, new TypeToken>>() { }.getType(), writer);
--- End diff --
Why need to encode again? You can just write the `jsonStr` to the file.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664790#comment-15664790
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87353516
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,30 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ * The log level for a logger name can be {@code null} except for the
root logger, which will reset the log level for
--- End diff --
What happen if the caller pass in `null` value for the root logger? Will it
throw exception? Or will it get ignore?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664791#comment-15664791
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87875415
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -56,24 +66,27 @@
public TwillContainerService(BasicTwillContext context, ContainerInfo
containerInfo, ZKClient zkClient,
RunId runId, TwillRunnableSpecification
specification, ClassLoader classLoader,
- Location applicationLocation) {
+ Location applicationLocation, Map logLevels) {
super(zkClient, runId, applicationLocation);
this.specification = specification;
this.classLoader = classLoader;
-this.containerLiveNodeData = createLiveNodeData(containerInfo);
+this.containerLiveNodeData = createLiveNodeData(containerInfo,
+isLoggerContext()
+ ? logLevels :
Collections.emptyMap());
this.context = context;
}
- private ContainerLiveNodeData createLiveNodeData(ContainerInfo
containerInfo) {
+ private ContainerLiveNodeData createLiveNodeData(ContainerInfo
containerInfo,
+ Map logLevels) {
// if debugging is enabled, log the port and register it in service
discovery.
String debugPort = System.getProperty("twill.debug.port");
if (debugPort != null) {
LOG.info("JVM is listening for debugger on port {}", debugPort);
}
return new ContainerLiveNodeData(containerInfo.getId(),
containerInfo.getHost().getCanonicalHostName(),
- debugPort);
+ debugPort, logLevels);
--- End diff --
I think we should filter out the one that has `null` value, since those are
representing reset.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664788#comment-15664788
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87353645
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillPreparer.java
---
@@ -227,15 +227,34 @@
TwillPreparer addSecureStore(SecureStore secureStore);
/**
- * Set the log level for Twill applications running in a container.
+ * Set the root log level for Twill applications in all containers.
*
* @param logLevel the {@link LogEntry.Level} that should be set.
* The level match the {@code Logback} levels.
* @return This {@link TwillPreparer}.
+ * @deprecated Use {@link #setLogLevels(Map)} with logger name
Logger.ROOT_LOGGER_NAME instead.
--- End diff --
Should be `{@link Logger#ROOT_LOGGER_NAME}`
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664789#comment-15664789
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87872004
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/TwillRunResourcesCodec.java
---
@@ -66,13 +67,14 @@ public JsonElement serialize(TwillRunResources src,
Type typeOfSrc, JsonSerializ
public TwillRunResources deserialize(JsonElement json, Type typeOfT,
JsonDeserializationContext
context) throws JsonParseException {
JsonObject jsonObj = json.getAsJsonObject();
+Map logLevels =
+ context.deserialize(jsonObj.get("logLevels"), new
TypeToken>() { }.getType());
--- End diff --
Since constants are defined, we should use those constants in
deserialization as well.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664786#comment-15664786
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87874961
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -605,5 +672,14 @@ public synchronized Integer getDebugPort() {
}
return dynamicDebugPort;
}
+
+@Override
+public synchronized Map getLogLevels() {
+ ContainerLiveNodeData liveData = controller.getLiveNodeData();
+ if (liveData != null) {
+return liveData.getLogLevels();
+ }
+ return new HashMap<>();
--- End diff --
return `Collections.emptyMap()`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664785#comment-15664785
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87863869
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillPreparer.java
---
@@ -227,15 +227,34 @@
TwillPreparer addSecureStore(SecureStore secureStore);
/**
- * Set the log level for Twill applications running in a container.
+ * Set the root log level for Twill applications in all containers.
*
* @param logLevel the {@link LogEntry.Level} that should be set.
* The level match the {@code Logback} levels.
* @return This {@link TwillPreparer}.
+ * @deprecated Use {@link #setLogLevels(Map)} with logger name
Logger.ROOT_LOGGER_NAME instead.
*/
+ @Deprecated
TwillPreparer setLogLevel(LogEntry.Level logLevel);
/**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return This {@link TwillPreparer}.
+ */
+ TwillPreparer setLogLevels(Map logLevels);
--- End diff --
Will this map allows `null` value? Unlike the dynamic case, it seems like
there is no point in allowing that, which means we should validate and document
it?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664784#comment-15664784
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87864069
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -90,8 +104,14 @@ public Integer getDebugPort() {
}
@Override
+ @Deprecated
public Level getLogLevel() {
-return logLevel;
+return getLogLevels().get(Logger.ROOT_LOGGER_NAME);
--- End diff --
So this may return `null`, right? Or can we guarantee that it won't be
`null` since we always know the root log level when a container starts?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651735#comment-15651735
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87254498
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/ServiceMain.java ---
@@ -264,6 +262,10 @@ protected String getLoggerLevel(Logger logger) {
return "OFF";
}
+ protected String getLogLevels() {
+return "";
--- End diff --
For a launch, the `TwillContainerService` is constructed before the run of
`doMain` method, which means the we have not called `configureLogger()` method
at that time so log level setting at constructor will not work.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651713#comment-15651713
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87253067
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,59 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevelsToDefault();
+
+ /**
+ * Reset the log levels of the given runnable to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * runnable name as the result.
+ */
+ Future resetRunnableLogLevelsToDefault(String runnableName);
--- End diff --
`resetLogLevels` will reset all log levels passed in runtime except the
root to null. If user want to reset log level for a particular logger, he can
use `setLogLevels(ImmutableMap.of(someloggerName, null))` to set the log level
since null is actually a valid log level for log back
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651612#comment-15651612
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87242632
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java
---
@@ -41,19 +46,15 @@
private final AtomicReference> services;
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources) {
-this(applicationId, masterResources, ImmutableMap.>of());
- }
-
- public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
- Map>
resources) {
-this(applicationId, masterResources, resources,
ImmutableList.of());
+this(applicationId, masterResources, Collections.>emptyMap(),
+ Collections.emptyList());
}
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
Map>
resources, List services) {
this.applicationId = applicationId;
this.appMasterResources = masterResources;
-this.usedResources = HashMultimap.create();
+this.usedResources =
Multimaps.synchronizedSetMultimap(HashMultimap.create());
--- End diff --
Is this change still necessary?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651607#comment-15651607
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87244879
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/ServiceMain.java ---
@@ -264,6 +262,10 @@ protected String getLoggerLevel(Logger logger) {
return "OFF";
}
+ protected String getLogLevels() {
+return "";
--- End diff --
Why we can't set the custom log levels at launch in the same programmatic
way as when a system message is received?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651608#comment-15651608
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87236796
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,59 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevelsToDefault();
+
+ /**
+ * Reset the log levels of the given runnable to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * runnable name as the result.
+ */
+ Future resetRunnableLogLevelsToDefault(String runnableName);
--- End diff --
Same here, call it `resetLogLevels(String runnableName)` to mirror the
`set` methods.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651609#comment-15651609
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87237480
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillPreparer.java
---
@@ -227,15 +227,52 @@
TwillPreparer addSecureStore(SecureStore secureStore);
/**
- * Set the log level for Twill applications running in a container.
+ * Set the root log level for Twill applications in all containers.
*
* @param logLevel the {@link LogEntry.Level} that should be set.
* The level match the {@code Logback} levels.
* @return This {@link TwillPreparer}.
+ * @deprecated Use {@link #setRootLogLevel(LogEntry.Level)} instead.
*/
+ @Deprecated
TwillPreparer setLogLevel(LogEntry.Level logLevel);
/**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel the {@link LogEntry.Level} that should be set.
+ * The level match the {@code Logback} levels.
+ * @return This {@link TwillPreparer}.
+ */
+ TwillPreparer setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel the {@link LogEntry.Level} that should be set.
+ * @return This {@link TwillPreparer}.
+ */
+ TwillPreparer setRootLogLevel(String runnableName, LogEntry.Level
logLevel);
--- End diff --
I prefer not to introduce the `setRootLogLevel` and the deprecated
`setLogLevel` replaced with call to `setLogLevel(Map)` with logger name
`Logger.ROOT_LOGGER_NAME` to set root logger log level.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651606#comment-15651606
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87242433
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/ContainerLiveNodeData.java
---
@@ -43,4 +51,15 @@ public String getHost() {
public String getDebugPort() {
return debugPort;
}
+
+ public Map getLogLevels() {
+return logLevels;
+ }
+
+ public void updateLogLevels(Map logLevels,
boolean isReset) {
+if (isReset) {
--- End diff --
This is not a good way overloading the responsibility of this method.
Better have a separate method for resetting (and that mirrors the actual
functionality being exposed through the public API).
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651615#comment-15651615
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87238388
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -30,17 +35,25 @@
private final int memoryMB;
private final String host;
private final Integer debugPort;
- private final Level logLevel;
+ private final Map logLevels;
+
+ /**
+ * Constructor to create an instance of {@link DefaultTwillRunResources}
with empty log levels.
+ */
+ public DefaultTwillRunResources(int instanceId, String containerId, int
cores, int memoryMB,
+ String host, Integer debugPort) {
+this(instanceId, containerId, cores, memoryMB, host, debugPort, new
HashMap());
--- End diff --
Use `Collections.emptyMap()` instead.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651610#comment-15651610
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87238505
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -129,7 +153,7 @@ public String toString() {
", memoryMB=" + memoryMB +
", host='" + host + '\'' +
", debugPort=" + debugPort +
- ", logLevel=" + logLevel +
+ ", rootLogLevel=" + logLevels.get(Logger.ROOT_LOGGER_NAME) +
--- End diff --
Why only has root log level? The `toString` should includes the whole
`logLevels` map.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651616#comment-15651616
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87237581
--- Diff:
twill-api/src/main/java/org/apache/twill/api/TwillRunResources.java ---
@@ -58,7 +60,18 @@
/**
* @return the enabled log level for the container where the runnable is
running in.
+ * @deprecated Use {@link #getRootLogLevel()} instead.
*/
+ @Deprecated
LogEntry.Level getLogLevel();
+ /**
+ * @return the enabled log level for the container where the runnable is
running in.
+ */
+ LogEntry.Level getRootLogLevel();
--- End diff --
Same reason as above, don't introduce this method just specialized for root
logger.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651617#comment-15651617 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r87242662 --- Diff: twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java --- @@ -17,17 +17,22 @@ */ package org.apache.twill.internal; +import ch.qos.logback.classic.Logger; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Multimaps; import com.google.common.collect.SetMultimap; import org.apache.twill.api.ResourceReport; import org.apache.twill.api.TwillRunResources; +import org.apache.twill.api.logging.LogEntry; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; --- End diff -- unused import? > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651614#comment-15651614
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87239528
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,59 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevelsToDefault();
+
+ /**
+ * Reset the log levels of the given runnable to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * runnable name as the result.
+ */
+ Future resetRunnableLogLevelsToDefault(String runnableName);
--- End diff --
Also, this reset everything? So there is no way to reset only a subset of
loggers?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651604#comment-15651604
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87237171
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
--- End diff --
In that case, why we have the `setRootLogLevel` methods above? I don't like
have two ways of doing the same task.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651605#comment-15651605
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87236703
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,59 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
+
+ /**
+ * Reset the log levels of all runnables to the default log level,
{@code LogEntry.Level.INFO}.
+ *
+ * @return A {@link Future} that will be completed when the set log
level operation has been done.
+ */
+ Future resetLogLevelsToDefault();
--- End diff --
Just call it `resetLogLevels`
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651613#comment-15651613
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r87239353
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java
---
@@ -170,6 +171,37 @@ public String apply(Set input) {
});
}
+ @Override
+ public Future setRootLogLevel(LogEntry.Level logLevel) {
+return
sendMessage(SystemMessages.setLogLevels(ImmutableMap.of(Logger.ROOT_LOGGER_NAME,
logLevel)), logLevel);
+ }
+
+ @Override
+ public Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel) {
+return sendMessage(SystemMessages.setLogLevels(runnableName,
ImmutableMap.of(Logger.ROOT_LOGGER_NAME, logLevel)),
+ logLevel);
+ }
+
+ @Override
+ public Future> setLogLevels(Map logLevels) {
+return sendMessage(SystemMessages.setLogLevels(logLevels), logLevels);
+ }
+
+ @Override
+ public Future> setLogLevels(String
runnableName,
+ Map runnableLogLevels) {
+return sendMessage(SystemMessages.setLogLevels(runnableName,
runnableLogLevels), runnableLogLevels);
+ }
+
+ @Override
+ public Future resetLogLevelsToDefault() {
+return sendMessage(SystemMessages.resetLogLevels(null), "all");
--- End diff --
This is inconsistent with the `setLogLevels` method, which doesn't require
explicitly passing in `null` for all runnables case.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651611#comment-15651611 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r87244253 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java --- @@ -17,12 +17,14 @@ */ package org.apache.twill.internal; +import ch.qos.logback.classic.Logger; --- End diff -- Unused import? > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631790#comment-15631790
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86290796
--- Diff:
twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelChangeTestRun.java ---
@@ -0,0 +1,249 @@
+/*
+ * 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.twill.yarn;
+
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
+import org.apache.twill.api.AbstractTwillRunnable;
+import org.apache.twill.api.ResourceReport;
+import org.apache.twill.api.TwillApplication;
+import org.apache.twill.api.TwillController;
+import org.apache.twill.api.TwillRunResources;
+import org.apache.twill.api.TwillSpecification;
+import org.apache.twill.api.logging.LogEntry;
+import org.apache.twill.api.logging.PrinterLogHandler;
+import org.apache.twill.common.Threads;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.PrintWriter;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Test changing log level for a twill runnable.
+ */
+public class LogLevelChangeTestRun extends BaseYarnTest {
+ public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.class);
+
+ /**
+ * Twill runnable.
+ */
+ public static final class LogLevelTestRunnable extends
AbstractTwillRunnable {
+public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.LogLevelTestRunnable.class);
+
+private volatile Thread runThread;
+
+@Override
+public void run() {
+ this.runThread = Thread.currentThread();
+
+ // check if the initial log level is DEBUG
+ Assert.assertTrue(LOG.isDebugEnabled() && !LOG.isTraceEnabled());
+
+ int i = 0;
+ while (!Thread.interrupted()) {
+if (i == 0 && !LOG.isDebugEnabled()) {
+ // check if the log level is changed to INFO
+ Assert.assertTrue(LOG.isInfoEnabled());
+ i++;
+}
+if (i == 1 && !LOG.isInfoEnabled()) {
+ // check if the log level is changed to WARN
+ Assert.assertTrue(LOG.isWarnEnabled());
+ i++;
+}
+
+try {
+ TimeUnit.MILLISECONDS.sleep(100);
+} catch (InterruptedException e) {
+ break;
+}
+ }
+}
+
+@Override
+public void stop() {
+ if (runThread != null) {
+runThread.interrupt();
+ }
+}
+ }
+
+ /**
+ * Second runnable.
+ */
+ public static final class LogLevelTestSecondRunnable extends
AbstractTwillRunnable {
+public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.LogLevelTestSecondRunnable.class);
+
+private volatile Thread runThread;
+
+@Override
+public void run() {
+ this.runThread = Thread.currentThread();
+
+ // check if the initial log level is DEBUG
+ Assert.assertTrue(LOG.isDebugEnabled() && !LOG.isTraceEnabled());
+
+ int i = 0;
+ while (!Thread.interrupted()) {
+if (i == 0 && !LOG.isDebugEnabled()) {
+ // check if the log level is changed to INFO
+ Assert.assertTrue(LOG.isInfoEnabled());
+ i++;
+}
+if (i == 1 && LOG.isDebugEnabled()) {
+ // check if the log level is changed to TRACE
+ Assert.assertTrue(LOG.isTraceEnabled());
+ i++;
+}
+
+try {
+ TimeUnit.MILLISECONDS.sle
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631720#comment-15631720
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86289119
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
---
@@ -72,7 +76,7 @@ public String getZkConnectStr() {
return zkConnectStr;
}
- public RunId getTwillRunId() {
+ public RunId getTwillAppRunId() {
--- End diff --
We don't have to since it is not in twill-api
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631701#comment-15631701
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86288660
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -475,6 +496,11 @@ private void sendMessage(final String runnableName,
final Message message,
Futures.addCallback(controller.sendMessage(message), new
FutureCallback() {
@Override
public void onSuccess(Message result) {
+//update log level information
--- End diff --
onSuccess here means that the message has been processed by the
TwillContainerService, and the log levels have been set up. If there is some
error setting up the log levels, an failed future with exception will be
returned to callback, so it will go to onFailure() method
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631691#comment-15631691
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86288445
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -575,18 +601,74 @@ private boolean removeContainerInfo(String
containerId) {
return false;
}
+ private Map>
copyLogLevels(Map> originalMap) {
+Map> result = new TreeMap<>();
+for (Map.Entry> entry :
originalMap.entrySet()) {
+ Map value = new TreeMap<>();
+ for (Map.Entry valueEntry :
entry.getValue().entrySet()) {
+value.put(valueEntry.getKey(), valueEntry.getValue());
+ }
+ result.put(entry.getKey(), value);
+}
+return result;
+ }
+
+ private void checkAndSetLogLevels(Message message, String runnableName) {
+if (message.getType() != Message.Type.SYSTEM
+ ||
!SystemMessages.LOG_LEVEL.equals(message.getCommand().getCommand())) {
+ return;
+}
+
+Map runnableLogLevels =
convertLogLevelValuesToLogEntry(message.getCommand().getOptions());
+if (!logLevels.containsKey(runnableName)) {
+ logLevels.put(runnableName, runnableLogLevels);
+} else {
+ logLevels.get(runnableName).putAll(runnableLogLevels);
+}
+ }
+
+ private Location saveLogLevels() {
--- End diff --
We save the log levels to file because we have no way to know if there is
message about log level change come before the launch of some containers(this
can happen when we change instance count or restart the container). So when a
message about log level change come, we will have it stored in the Map in
`RunningContainers`. And if there is a container launch afterwards, we will
check if there is update in the log levels, and localize it for the container
if necessary.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631657#comment-15631657
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287618
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
---
@@ -89,8 +91,7 @@ public String getRmSchedulerAddr() {
return rmSchedulerAddr;
}
- @Nullable
- public LogEntry.Level getLogLevel() {
-return logLevel;
+ public Map> getLogLevels() {
+return logLevels;
}
--- End diff --
ok
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631651#comment-15631651
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287548
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
--- End diff --
So again, does success mean the message was successfully sent? Or
successfully processed?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631644#comment-15631644
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287397
--- Diff:
twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelChangeTestRun.java ---
@@ -0,0 +1,249 @@
+/*
+ * 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.twill.yarn;
+
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
+import org.apache.twill.api.AbstractTwillRunnable;
+import org.apache.twill.api.ResourceReport;
+import org.apache.twill.api.TwillApplication;
+import org.apache.twill.api.TwillController;
+import org.apache.twill.api.TwillRunResources;
+import org.apache.twill.api.TwillSpecification;
+import org.apache.twill.api.logging.LogEntry;
+import org.apache.twill.api.logging.PrinterLogHandler;
+import org.apache.twill.common.Threads;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.PrintWriter;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Test changing log level for a twill runnable.
+ */
+public class LogLevelChangeTestRun extends BaseYarnTest {
+ public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.class);
+
+ /**
+ * Twill runnable.
+ */
+ public static final class LogLevelTestRunnable extends
AbstractTwillRunnable {
+public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.LogLevelTestRunnable.class);
+
+private volatile Thread runThread;
+
+@Override
+public void run() {
+ this.runThread = Thread.currentThread();
+
+ // check if the initial log level is DEBUG
+ Assert.assertTrue(LOG.isDebugEnabled() && !LOG.isTraceEnabled());
+
+ int i = 0;
+ while (!Thread.interrupted()) {
+if (i == 0 && !LOG.isDebugEnabled()) {
+ // check if the log level is changed to INFO
+ Assert.assertTrue(LOG.isInfoEnabled());
+ i++;
+}
+if (i == 1 && !LOG.isInfoEnabled()) {
+ // check if the log level is changed to WARN
+ Assert.assertTrue(LOG.isWarnEnabled());
+ i++;
+}
+
+try {
+ TimeUnit.MILLISECONDS.sleep(100);
+} catch (InterruptedException e) {
+ break;
+}
+ }
+}
+
+@Override
+public void stop() {
+ if (runThread != null) {
+runThread.interrupt();
+ }
+}
+ }
+
+ /**
+ * Second runnable.
+ */
+ public static final class LogLevelTestSecondRunnable extends
AbstractTwillRunnable {
+public static final Logger LOG =
LoggerFactory.getLogger(LogLevelChangeTestRun.LogLevelTestSecondRunnable.class);
+
+private volatile Thread runThread;
+
+@Override
+public void run() {
+ this.runThread = Thread.currentThread();
+
+ // check if the initial log level is DEBUG
+ Assert.assertTrue(LOG.isDebugEnabled() && !LOG.isTraceEnabled());
+
+ int i = 0;
+ while (!Thread.interrupted()) {
+if (i == 0 && !LOG.isDebugEnabled()) {
+ // check if the log level is changed to INFO
+ Assert.assertTrue(LOG.isInfoEnabled());
+ i++;
+}
+if (i == 1 && LOG.isDebugEnabled()) {
+ // check if the log level is changed to TRACE
+ Assert.assertTrue(LOG.isTraceEnabled());
+ i++;
+}
+
+try {
+ TimeUnit.MILLISECONDS.sleep(100
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631643#comment-15631643
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287193
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java
---
@@ -123,6 +142,9 @@ protected void doStart() throws Exception {
runnable = Instances.newInstance((Class) runnableClass);
runnable.initialize(context);
+if (!setLogLevels(Maps.transformValues(logLevels,
Functions.toStringFunction( {
+ LOG.error("LoggerFactory is not a logback LoggerContext, cannot
change log level");
--- End diff --
seems that the error logging belongs into the setLogLevels() method.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631642#comment-15631642
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86286972
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -475,6 +496,11 @@ private void sendMessage(final String runnableName,
final Message message,
Futures.addCallback(controller.sendMessage(message), new
FutureCallback() {
@Override
public void onSuccess(Message result) {
+//update log level information
--- End diff --
onSuccess here really means that the message has been sent to the runnable
successfully, right? But it does not reflect the actual state of the runnable
itself? It that on purpose?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631640#comment-15631640
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287245
--- Diff:
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---
@@ -296,8 +298,42 @@ public TwillPreparer addSecureStore(SecureStore
secureStore) {
@Override
public TwillPreparer setLogLevel(LogEntry.Level logLevel) {
+return setRootLogLevel(logLevel);
+ }
+
+ @Override
+ public TwillPreparer setRootLogLevel(LogEntry.Level logLevel) {
Preconditions.checkNotNull(logLevel);
this.logLevel = logLevel;
+saveLogLevels(logLevel);
+return this;
+ }
+
+ @Override
+ public TwillPreparer setRootLogLevel(String runnableName, LogEntry.Level
logLevel) {
+setLogLevels(runnableName, ImmutableMap.of(Logger.ROOT_LOGGER_NAME,
logLevel));
+return this;
+ }
+
+ @Override
+ public TwillPreparer setLogLevels(Map logLevels)
{
+Preconditions.checkNotNull(logLevels);
+if (logLevels.containsKey(Logger.ROOT_LOGGER_NAME)) {
+ this.logLevel = logLevels.get(Logger.ROOT_LOGGER_NAME);
+}
+for (String runnableName : twillSpec.getRunnables().keySet()) {
+ saveLogLevels(runnableName, logLevels);
+}
+return this;
+ }
+
+ @Override
+ public TwillPreparer setLogLevels(String runnableName, Map runnableLogLeves) {
--- End diff --
typo: runnableLogLevels
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631641#comment-15631641
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86287011
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java
---
@@ -575,18 +601,74 @@ private boolean removeContainerInfo(String
containerId) {
return false;
}
+ private Map>
copyLogLevels(Map> originalMap) {
+Map> result = new TreeMap<>();
+for (Map.Entry> entry :
originalMap.entrySet()) {
+ Map value = new TreeMap<>();
+ for (Map.Entry valueEntry :
entry.getValue().entrySet()) {
+value.put(valueEntry.getKey(), valueEntry.getValue());
+ }
+ result.put(entry.getKey(), value);
+}
+return result;
+ }
+
+ private void checkAndSetLogLevels(Message message, String runnableName) {
+if (message.getType() != Message.Type.SYSTEM
+ ||
!SystemMessages.LOG_LEVEL.equals(message.getCommand().getCommand())) {
+ return;
+}
+
+Map runnableLogLevels =
convertLogLevelValuesToLogEntry(message.getCommand().getOptions());
+if (!logLevels.containsKey(runnableName)) {
+ logLevels.put(runnableName, runnableLogLevels);
+} else {
+ logLevels.get(runnableName).putAll(runnableLogLevels);
+}
+ }
+
+ private Location saveLogLevels() {
--- End diff --
Can you explain why the log levels have to be saved to a file? It is not
clear to me.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631579#comment-15631579
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86286160
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
+ }
+
+ if (!(type.getType() instanceof ParameterizedType)) {
+return null;
+ }
+
+ TypeToken valueType = TypeToken.get(((ParameterizedType)
type.getType()).getActualTypeArguments()[0]);
+ return (TypeAdapter) collectionAdapter(gson, valueType);
+}
+
+private TypeAdapter> collectionAdapter(Gson gson,
TypeToken valueType) {
+ final TypeAdapter valueAdapter = gson.getAdapter(valueType);
+ return new TypeAdapter>() {
+@Override
+public void write(JsonWriter writer, @Nullable Collection
collection) throws IOException {
+ if (collection == null) {
+writer.nullValue();
+return;
+ }
+ writer.beginArray();
+ for (V value : collection) {
+valueAdapter.write(writer, value);
+ }
+ writer.endArray();
+}
+
+@Override
+public Collection read(JsonReader reader) throws IOException {
--- End diff --
Added
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631578#comment-15631578
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86286137
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java
---
@@ -158,4 +174,31 @@ public String toString() {
", services=" + services +
'}';
}
+
+ private TwillRunResources wrapTwillRunResources(final String
runnableName, TwillRunResources resources) {
--- End diff --
The `logLevels` for all running containers are stored in `ResourceReport`,
and `ResourceReport` have a map containing the `TwillRunResources` for each
running runnable. We want to return the correct log levels for each runnable.
So we wrap the `TwillRunResources` with `ForwardingTwillRunResources`, which
has a delegation and we override the `getLogLevels` methods to get the log
levels from the map in `ResourceReport`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631542#comment-15631542
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86285363
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
--- End diff --
This factory is specifically for serializing`Collection`
since Gson will not catch the inner class for it. So we only need to create the
TypeAdapter when we encounter `Collection`. Actually the
implementation is simulated from `TwillRuntimeSpecificationAdapter`, which
create factory to deal with similar situation,
https://github.com/apache/twill/blob/master/twill-core/src/main/java/org/apache/twill/internal/json/TwillRuntimeSpecificationAdapter.java#L106
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631426#comment-15631426
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86282926
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
--- End diff --
For this PR, we do not have that functionality. You will have to change the
log level of the particular logger name again to have the log level back.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631423#comment-15631423
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86282861
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
--- End diff --
Yes, you can set the root log level by calling like:
`setLogLevels(ImmutableMap.of("ROOT", TRACE))`
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631410#comment-15631410
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86282520
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
--- End diff --
We are sending messages using ZooKeeper. The future will be completed once
the message has been processed. Or if there is problem sending the message, an
exception will be added as the value of the future.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631269#comment-15631269
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86276109
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
+ }
+
+ if (!(type.getType() instanceof ParameterizedType)) {
+return null;
+ }
+
+ TypeToken valueType = TypeToken.get(((ParameterizedType)
type.getType()).getActualTypeArguments()[0]);
+ return (TypeAdapter) collectionAdapter(gson, valueType);
+}
+
+private TypeAdapter> collectionAdapter(Gson gson,
TypeToken valueType) {
+ final TypeAdapter valueAdapter = gson.getAdapter(valueType);
+ return new TypeAdapter>() {
+@Override
+public void write(JsonWriter writer, @Nullable Collection
collection) throws IOException {
+ if (collection == null) {
+writer.nullValue();
+return;
+ }
+ writer.beginArray();
+ for (V value : collection) {
+valueAdapter.write(writer, value);
+ }
+ writer.endArray();
+}
+
+@Override
+public Collection read(JsonReader reader) throws IOException {
--- End diff --
this is a @Nullable
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631266#comment-15631266
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86273964
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
--- End diff --
It's not clear to me why these need to return a future (or anything at
all). Is it to be able to communicate an error/exception in case the setting of
the log level fails? So, when it returns it is guaranteed that all runnables
have actually adapted the new log level?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631267#comment-15631267
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275651
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
---
@@ -89,8 +91,7 @@ public String getRmSchedulerAddr() {
return rmSchedulerAddr;
}
- @Nullable
- public LogEntry.Level getLogLevel() {
-return logLevel;
+ public Map> getLogLevels() {
+return logLevels;
}
--- End diff --
Same here, existing clients might break if you remove a method. Or is this
internal?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631270#comment-15631270
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275966
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
--- End diff --
this is an illegal argument? Or is this actually expected to happen? If so,
you should annotate the method as @Nullable. If not, you should throw an
exception.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631274#comment-15631274
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86274149
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
--- End diff --
does this also allow to set the root log level? Or does that have to happen
in a separate setRootLogLevel() call?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631265#comment-15631265
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86274829
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java
---
@@ -137,7 +138,7 @@ public final synchronized ServiceDiscovered
discoverService(String serviceName)
@Override
public final ListenableFuture> restartInstances(Map> runnableToInstanceIds) {
+? extends Set> runnableToInstanceIds) {
--- End diff --
I agree the formatting is odd. Better move the entire Map<...> to the next
line
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631273#comment-15631273
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275606
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
---
@@ -72,7 +76,7 @@ public String getZkConnectStr() {
return zkConnectStr;
}
- public RunId getTwillRunId() {
+ public RunId getTwillAppRunId() {
--- End diff --
For compatibility, can you keep the existing method as deprecated?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631277#comment-15631277
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86274641
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
+
+ /**
+ * Set the log levels for requested logger names for Twill applications
running in a container.
+ *
+ * @param logLevels The {@link Map} contains the requested logger names
and log levels that need to be set.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(Map logLevels);
+
+ /**
+ * Set the log levels for requested logger names for a {@link
TwillRunnable}.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevelsForRunnable The {@link Map} contains the requested
logger name and log level that
+ * need to be changed.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry the
+ * {@link Map} of log levels as the result.
+ */
+ Future> setLogLevels(String runnableName,
+ Map logLevelsForRunnable);
--- End diff --
Is there a way to reset the log level for a particular logger name? That
is, after I have set that logger to TRACE for some time, I have found all
information I need and I want it to go back to the root log level (or whatever
it would do if I had never set the log level for this logger). That also means
that if I change the root log level again, it will be applied to this logger.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631272#comment-15631272
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86274401
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,44 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(LogEntry.Level logLevel);
+
+ /**
+ * Set the root log level for a particular runnable.
+ *
+ * @param runnableName The name of the runnable to set the log level.
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the log level as a result.
+ */
+ Future setRootLogLevel(String runnableName,
LogEntry.Level logLevel);
--- End diff --
question about race conditions. Suppose I have an app that has N runnables,
and I set them all to DEBUG because there was a problem. Now I have identified
which runnable is the culprit, so I want to reset all others to WARN and the
suspicious one to TRACE. It looks like, for this, I need to make two calls:
```
setRootLogLevel(WARN);
setRootLogLevel("suspiciousRunnable", TRACE);
```
The suspicious runnable will be affected by both calls. Is it guaranteed
that after all, it will be on TRACE? That is, the order of calls is preserved?
Or what would be the correct way to do this?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631264#comment-15631264
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275759
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
+ }
+
+ if (!(type.getType() instanceof ParameterizedType)) {
+return null;
+ }
+
+ TypeToken valueType = TypeToken.get(((ParameterizedType)
type.getType()).getActualTypeArguments()[0]);
+ return (TypeAdapter) collectionAdapter(gson, valueType);
+}
+
+private TypeAdapter> collectionAdapter(Gson gson,
TypeToken valueType) {
+ final TypeAdapter valueAdapter = gson.getAdapter(valueType);
+ return new TypeAdapter>() {
+@Override
+public void write(JsonWriter writer, @Nullable Collection
collection) throws IOException {
+ if (collection == null) {
+writer.nullValue();
+return;
+ }
+ writer.beginArray();
+ for (V value : collection) {
+valueAdapter.write(writer, value);
+ }
+ writer.endArray();
+}
+
+@Override
+public Collection read(JsonReader reader) throws IOException {
+ if (reader.peek() == JsonToken.NULL) {
+reader.nextNull();
+return null;
+ }
+ if (reader.peek() != JsonToken.BEGIN_ARRAY) {
+return null;
--- End diff --
shouldn't this throw an exception? It is a violation of the codec.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631275#comment-15631275
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275201
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java
---
@@ -158,4 +174,31 @@ public String toString() {
", services=" + services +
'}';
}
+
+ private TwillRunResources wrapTwillRunResources(final String
runnableName, TwillRunResources resources) {
--- End diff --
can you explain what this is for? It's not obvious to me. I think it
deserves some commenting.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631271#comment-15631271
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86276202
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/TwillRunResourcesCodec.java
---
@@ -55,9 +58,11 @@ public JsonElement serialize(TwillRunResources src, Type
typeOfSrc, JsonSerializ
if (src.getDebugPort() != null) {
json.addProperty(DEBUG_PORT, src.getDebugPort());
}
-if (src.getLogLevel() != null) {
- json.addProperty(LOG_LEVEL, src.getLogLevel().toString());
+if (src.getRootLogLevel() != null) {
--- End diff --
is this needed? It appears to me that the root log level is already in the
src.getLogLevels()
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631268#comment-15631268
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86276224
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/TwillRunResourcesCodec.java
---
@@ -55,9 +58,11 @@ public JsonElement serialize(TwillRunResources src, Type
typeOfSrc, JsonSerializ
if (src.getDebugPort() != null) {
json.addProperty(DEBUG_PORT, src.getDebugPort());
}
-if (src.getLogLevel() != null) {
- json.addProperty(LOG_LEVEL, src.getLogLevel().toString());
+if (src.getRootLogLevel() != null) {
--- End diff --
and the deserialize is not reading it?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631276#comment-15631276
]
ASF GitHub Bot commented on TWILL-138:
--
Github user anew commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r86275989
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/ResourceReportAdapter.java
---
@@ -59,4 +72,61 @@ public ResourceReport fromJson(String json) {
public ResourceReport fromJson(Reader reader) {
return gson.fromJson(reader, ResourceReport.class);
}
+
+ /**
+ * A type adapter for serializing {@code Collection}
correctly
+ */
+ private static final class ResourceReportTypeAdapterFactory implements
TypeAdapterFactory {
+
+@SuppressWarnings("unchecked")
+@Override
+public TypeAdapter create(Gson gson, TypeToken type) {
+ if (!Collection.class.isAssignableFrom(type.getRawType())) {
+return null;
+ }
+
+ if (!(type.getType() instanceof ParameterizedType)) {
+return null;
--- End diff --
ditto
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15577077#comment-15577077
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83522325
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/json/TwillRunResourcesCodec.java
---
@@ -55,9 +58,11 @@ public JsonElement serialize(TwillRunResources src, Type
typeOfSrc, JsonSerializ
if (src.getDebugPort() != null) {
json.addProperty(DEBUG_PORT, src.getDebugPort());
}
-if (src.getLogLevel() != null) {
- json.addProperty(LOG_LEVEL, src.getLogLevel().toString());
+if (src.getRootLogLevel() != null) {
+ json.addProperty(ROOT_LOG_LEVEL, src.getRootLogLevel().name());
}
+json.add(LOG_LEVELS, context.serialize(src.getLogLevels(),
+ new TypeToken>() { }.getType()));
--- End diff --
The type should be `Map`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576951#comment-15576951
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83519663
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java
---
@@ -39,25 +44,33 @@
private final TwillRunResources appMasterResources;
private final String applicationId;
private final AtomicReference> services;
+ private final Map> logLevels;
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources) {
-this(applicationId, masterResources, ImmutableMap.>of());
- }
-
- public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
- Map>
resources) {
-this(applicationId, masterResources, resources,
ImmutableList.of());
+this(applicationId, masterResources, Collections.>emptyMap(),
+ Collections.emptyList());
}
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
Map>
resources, List services) {
this.applicationId = applicationId;
this.appMasterResources = masterResources;
-this.usedResources = HashMultimap.create();
+this.usedResources =
Multimaps.synchronizedSetMultimap(HashMultimap.create());
for (Map.Entry> entry :
resources.entrySet()) {
this.usedResources.putAll(entry.getKey(), entry.getValue());
}
this.services = new AtomicReference<>(services);
+this.logLevels = new ConcurrentHashMap<>();
+ }
+
+ public synchronized void setRunnableLogLevels(String runnableName,
Map runnableLogLevels) {
--- End diff --
Also, maybe good to just return if the `runnableLogLevels` map is empty.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576948#comment-15576948
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83519594
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java
---
@@ -39,25 +44,33 @@
private final TwillRunResources appMasterResources;
private final String applicationId;
private final AtomicReference> services;
+ private final Map> logLevels;
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources) {
-this(applicationId, masterResources, ImmutableMap.>of());
- }
-
- public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
- Map>
resources) {
-this(applicationId, masterResources, resources,
ImmutableList.of());
+this(applicationId, masterResources, Collections.>emptyMap(),
+ Collections.emptyList());
}
public DefaultResourceReport(String applicationId, TwillRunResources
masterResources,
Map>
resources, List services) {
this.applicationId = applicationId;
this.appMasterResources = masterResources;
-this.usedResources = HashMultimap.create();
+this.usedResources =
Multimaps.synchronizedSetMultimap(HashMultimap.create());
for (Map.Entry> entry :
resources.entrySet()) {
this.usedResources.putAll(entry.getKey(), entry.getValue());
}
this.services = new AtomicReference<>(services);
+this.logLevels = new ConcurrentHashMap<>();
+ }
+
+ public synchronized void setRunnableLogLevels(String runnableName,
Map runnableLogLevels) {
--- End diff --
better name it as `updateRunnableLogLevels`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576946#comment-15576946
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83519511
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -90,8 +103,18 @@ public Integer getDebugPort() {
}
@Override
+ @Deprecated
public Level getLogLevel() {
-return logLevel;
+return getRootLogLevel();
+ }
+
+ @Override
+ public Level getRootLogLevel() {
+return getLogLevels().get(Logger.ROOT_LOGGER_NAME);
+ }
+
+ public Map getLogLevels() {
--- End diff --
Add `@Override`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576246#comment-15576246
]
ASF GitHub Bot commented on TWILL-138:
--
Github user yaojiefeng commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83485638
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,34 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the {@link Map} of the log level as a result with ROOT as the
logger name and the log level as value.
+ */
+ Future> setLogLevel(LogEntry.Level logLevel);
--- End diff --
For a particular runnable, user can use setLogLevels(runnableName,
ImmutableMap.of("ROOT", loglevel)) to change the root log level.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576114#comment-15576114 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r83471846 --- Diff: twill-api/src/main/java/org/apache/twill/api/TwillPreparer.java --- @@ -227,7 +227,7 @@ TwillPreparer addSecureStore(SecureStore secureStore); /** - * Set the log level for Twill applications running in a container. + * Set the root log level for Twill applications in all containers. --- End diff -- Same as above, do we have a way to set the root log level for a particular runnablE? > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576111#comment-15576111
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83471666
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,34 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the {@link Map} of the log level as a result with ROOT as the
logger name and the log level as value.
+ */
+ Future> setLogLevel(LogEntry.Level logLevel);
--- End diff --
Also, for the `Future`, it seems like it's better just to carry
`LogEntry.Level`.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576116#comment-15576116
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83472057
--- Diff:
twill-api/src/main/java/org/apache/twill/api/TwillRunResources.java ---
@@ -59,6 +61,16 @@
/**
* @return the enabled log level for the container where the runnable is
running in.
*/
+ @Deprecated
--- End diff --
Why deprecating a method, also add a `@deprecated` in the javadoc section
to tell what's the alternative. E.g.
```
/**
*
* @deprecated Use {@link #getRootLogLevel()} instead.
*/
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576113#comment-15576113
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83471522
--- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java
---
@@ -89,4 +90,34 @@
* @return A {@link Future} that will be completed when the restart
operation has been done.
*/
Future restartInstances(String runnable, int instanceId, int...
moreInstanceIds);
+
+ /**
+ * Set the root log level for Twill applications in all containers.
+ *
+ * @param logLevel The log level for the root logger to change.
+ * @return A {@link Future} that will be completed when the set log
level operation has been done. It will carry
+ * the {@link Map} of the log level as a result with ROOT as the
logger name and the log level as value.
+ */
+ Future> setLogLevel(LogEntry.Level logLevel);
--- End diff --
So there is no way to set the root log level for a particular runnable?
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576112#comment-15576112 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r83471254 --- Diff: twill-api/src/main/java/org/apache/twill/api/ResourceReport.java --- @@ -61,4 +63,11 @@ * @return list of services of the application master. */ List getServices(); + + /** + * Get the map of the log levels enabled for the twill application. + * + * @return the map of the log level arguments of the twill application + */ + Map> getLogLevels(); --- End diff -- Why have this method? The log levels are already contained inside individual `TwillRunResources`. > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576115#comment-15576115
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83472284
--- Diff:
twill-api/src/main/java/org/apache/twill/internal/DefaultTwillRunResources.java
---
@@ -129,7 +152,7 @@ public String toString() {
", memoryMB=" + memoryMB +
", host='" + host + '\'' +
", debugPort=" + debugPort +
- ", logLevel=" + logLevel +
+ ", rootLogLevel=" + getRootLogLevel() +
--- End diff --
Use the `logLevels` field instead of calling the get method to be
consistent.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15570256#comment-15570256
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83112064
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/utils/LogLevelUtil.java ---
@@ -0,0 +1,56 @@
+/*
+ * 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.twill.internal.utils;
+
+import com.google.common.collect.Maps;
+import org.apache.twill.api.logging.LogEntry;
+
+import java.util.Map;
+
+/**
+ * Collections of helper functions for log level change.
+ */
+public final class LogLevelUtil {
+
+ /**
+ * Convert the log level argument value type to string.
+ */
+ public static Map
convertLogLevelValuesToString(Map logLevels) {
--- End diff --
No need to have this method. Just do a
```Maps.transformValues(logLevels, Functions.toStringFunction())```
when needed.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15570263#comment-15570263
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83112357
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/utils/LogLevelUtil.java ---
@@ -0,0 +1,56 @@
+/*
+ * 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.twill.internal.utils;
+
+import com.google.common.collect.Maps;
+import org.apache.twill.api.logging.LogEntry;
+
+import java.util.Map;
+
+/**
+ * Collections of helper functions for log level change.
+ */
+public final class LogLevelUtil {
+
+ /**
+ * Convert the log level argument value type to string.
+ */
+ public static Map
convertLogLevelValuesToString(Map logLevels) {
+return Maps.transformEntries(logLevels, new
Maps.EntryTransformer() {
+ @Override
+ public String transformEntry(String loggerName, LogEntry.Level
level) {
+return level.name();
+ }
+});
+ }
+
+ /**
+ * Convert the log level argument type to LogEntry.Level
+ */
+ public static Map
convertLogLevelValuesToLogEntry(Map logLevels) {
--- End diff --
Seems like the only usage is in `RunningContainers`. Move it there as a
private method.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15570254#comment-15570254
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83109946
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/state/SystemMessages.java ---
@@ -68,6 +73,31 @@ public static Message updateRunnablesInstances(Command
updateCommand) {
return new SimpleMessage(Message.Type.SYSTEM, Message.Scope.RUNNABLES,
null, updateCommand);
}
+ /**
+ * Helper method to get System {@link Message} for changing the log
levels for all runnables.
+ *
+ * @param logLevels The log levels to be changed.
+ * @return An instance of System {@link Message} to change the log
levels.
+ */
+ public static Message setLogLevels(Map
logLevels) {
+return setLogLevels(null, logLevels);
+ }
+
+ /**
+ * Helper method to get System {@link Message} for changing the log
levels for one or all runnables.
+ *
+ * @param runnableName The name of the runnable to set the log level,
ALL if apply to all runnables.
+ * @param logLevels The log levels to be changed.
+ * @return An instance of System {@link Message} to change the log
levels.
+ */
+ public static Message setLogLevels(String runnableName, Map logLevels) {
--- End diff --
Also document what does null mean.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[ https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15570259#comment-15570259 ] ASF GitHub Bot commented on TWILL-138: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r83116780 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java --- @@ -200,7 +201,9 @@ private RunningContainers initRunningContainers(ContainerId appMasterContainerId Integer.parseInt(System.getenv(EnvKeys.YARN_CONTAINER_MEMORY_MB)), appMasterHost, null, null); String appId = appMasterContainerId.getApplicationAttemptId().getApplicationId().toString(); -return new RunningContainers(appId, appMasterResources, zkClient); +return new RunningContainers(appId, appMasterResources, zkClient, + twillRuntimeSpec.getLogLevels(), applicationLocation, + new ArrayList<>(twillSpec.getRunnables().keySet())); --- End diff -- No need to copy since twill spec is immutable. > Runtime change of Application runnable log level > > > Key: TWILL-138 > URL: https://issues.apache.org/jira/browse/TWILL-138 > Project: Apache Twill > Issue Type: New Feature > Components: core >Reporter: Henry Saputra > > With TWILL-24 is supported for setting log level when starting the Twill > application, next enhancement is to able to send command to update the > aggregate log level for a particular runnable in a Twill application. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TWILL-138) Runtime change of Application runnable log level
[
https://issues.apache.org/jira/browse/TWILL-138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15570258#comment-15570258
]
ASF GitHub Bot commented on TWILL-138:
--
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/14#discussion_r83116829
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
---
@@ -951,4 +958,34 @@ public void run() {
}
};
}
+
+ /**
+ * Attempt to change the log level from a runnable or all runnables.
+ *
+ * @return {@code true} if the message requests changing log levels and
{@code false} otherwise.
+ */
+ private boolean handleLogLevelChange(final Message message, final
Runnable completion) {
--- End diff --
Seems like the `final` is unnecessary.
> Runtime change of Application runnable log level
>
>
> Key: TWILL-138
> URL: https://issues.apache.org/jira/browse/TWILL-138
> Project: Apache Twill
> Issue Type: New Feature
> Components: core
>Reporter: Henry Saputra
>
> With TWILL-24 is supported for setting log level when starting the Twill
> application, next enhancement is to able to send command to update the
> aggregate log level for a particular runnable in a Twill application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
