You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by uce <gi...@git.apache.org> on 2016/11/29 17:34:52 UTC
[GitHub] flink pull request #2899: Various logging improvements
GitHub user uce opened a pull request:
https://github.com/apache/flink/pull/2899
Various logging improvements
I would like to include the following fixes for the 1.1.4 release. In general, I try to improve the debugging experience with this.
1. **Reduce log pollution**: Some debug logs were very noisy and actually dominate the logs although they provide little value.
- Log heartbeats on TRACE level
- Don't log InputChannelDeploymentDescriptor
- Decrease HadoopFileSystem logging
2. **Improve log messages**: Some existing debug log messages were not that helpful.
- Log GlobalConfiguration loaded properties on INFO level
- Add TaskState toString
- Add more detailed log messages to HA job graph store
3. **Improve existing logger configuration templates**: The existing template simply configured the appenders and left everything except the root logger to the user. I changed it to be more fine grained (root logger, Flink, common libs/connectors). The goal is that users trying to DEBUG Flink, don't end up with too many unrelated log messages.
/cc @tillrohrmann
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/uce/flink logging
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2899.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2899
----
commit 20ceaef71e8d10dbaf0cb01b8548fc3870d5ebac
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T15:00:02Z
[FLINK-5201] [logging] Log loaded config properties on INFO level
commit bb837cece0f0945ab05e71eda5c3a728a64b390b
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T15:04:48Z
[FLINK-5196] [logging] Don't log InputChannelDeploymentDescriptor
commit 6a21e85fc7d99ba687753b4989bb25eb19aaa3af
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T16:15:27Z
[FLINK-5194] [logging] Log heartbeats on TRACE level
commit 4a91b78e52c440d5c736789cea0f8d80968339f8
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T15:15:30Z
[FLINK-5198] [logging] Improve TaskState toString
commit 5a5a78a15c2c3a0b3d82446b98c0c73c26cb4a4d
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T15:35:14Z
[FLINK-5199] [logging] Improve logging in ZooKeeperSubmittedJobGraphStore
commit 83ef198008e4dd84a4876b7a8cefc1f870ebe0a2
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T16:08:53Z
[FLINK-5207] [logging] Decrease HadoopFileSystem logging
commit 98c63af63036178e9c8423b99360f42d18b2e388
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-11-29T16:14:23Z
[FLINK-5192] [logging] Improve log config templates
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90228943
--- Diff: flink-dist/src/main/flink-bin/conf/log4j.properties ---
@@ -16,14 +16,26 @@
# limitations under the License.
################################################################################
+# This affects logging for both user code and Flink
log4j.rootLogger=INFO, file
+# Uncomment this if you want to _only_ change Flink's logging
+#log4j.logger.org.apache.flink=INFO
+
+# The following lines keep the log level of common libraries/connectors on
+# log level INFO. The root logger does not override this. You have to manually
+# change the log levels here.
+log4j.logger.akka=INFO
+log4j.logger.org.apache.kafka=INFO
+log4j.logger.org.apache.hadoop=INFO
--- End diff --
Does this also work for the shaded hadoop dependencies?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90235045
--- Diff: flink-dist/src/main/flink-bin/conf/log4j.properties ---
@@ -16,14 +16,26 @@
# limitations under the License.
################################################################################
+# This affects logging for both user code and Flink
log4j.rootLogger=INFO, file
+# Uncomment this if you want to _only_ change Flink's logging
+#log4j.logger.org.apache.flink=INFO
+
+# The following lines keep the log level of common libraries/connectors on
+# log level INFO. The root logger does not override this. You have to manually
+# change the log levels here.
+log4j.logger.akka=INFO
+log4j.logger.org.apache.kafka=INFO
+log4j.logger.org.apache.hadoop=INFO
--- End diff --
Ah, now I remember ;) This is indeed shady business. But good to know that we can keep it as is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90229390
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/fs/hdfs/HadoopFileSystem.java ---
@@ -211,17 +209,14 @@ public HadoopFileSystem(Class<? extends org.apache.hadoop.fs.FileSystem> fsClass
if (new File(possibleHadoopConfPath).exists()) {
if (new File(possibleHadoopConfPath + "/core-site.xml").exists()) {
retConf.addResource(new org.apache.hadoop.fs.Path(possibleHadoopConfPath + "/core-site.xml"));
-
- if (LOG.isDebugEnabled()) {
- LOG.debug("Adding " + possibleHadoopConfPath + "/core-site.xml to hadoop configuration");
- }
+ } else {
+ LOG.debug("File " + possibleHadoopConfPath + "/core-site.xml not found.");
--- End diff --
`{}` for variables.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by uce <gi...@git.apache.org>.
Github user uce closed the pull request at:
https://github.com/apache/flink/pull/2899
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2899: Various logging improvements
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/2899
Added `FlinkUntypedActor` as well and addressed the comments, Till. I would like to merge this now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90233907
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/fs/hdfs/HadoopFileSystem.java ---
@@ -211,17 +209,14 @@ public HadoopFileSystem(Class<? extends org.apache.hadoop.fs.FileSystem> fsClass
if (new File(possibleHadoopConfPath).exists()) {
if (new File(possibleHadoopConfPath + "/core-site.xml").exists()) {
retConf.addResource(new org.apache.hadoop.fs.Path(possibleHadoopConfPath + "/core-site.xml"));
-
- if (LOG.isDebugEnabled()) {
- LOG.debug("Adding " + possibleHadoopConfPath + "/core-site.xml to hadoop configuration");
- }
+ } else {
+ LOG.debug("File " + possibleHadoopConfPath + "/core-site.xml not found.");
--- End diff --
Sure
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90234726
--- Diff: flink-dist/src/main/flink-bin/conf/log4j.properties ---
@@ -16,14 +16,26 @@
# limitations under the License.
################################################################################
+# This affects logging for both user code and Flink
log4j.rootLogger=INFO, file
+# Uncomment this if you want to _only_ change Flink's logging
+#log4j.logger.org.apache.flink=INFO
+
+# The following lines keep the log level of common libraries/connectors on
+# log level INFO. The root logger does not override this. You have to manually
+# change the log levels here.
+log4j.logger.akka=INFO
+log4j.logger.org.apache.kafka=INFO
+log4j.logger.org.apache.hadoop=INFO
--- End diff --
We are not relocating Hadoop, so this configuration line is correct.
As you can see from our logs (the first line usually):
`org.apache.hadoop.util.NativeCodeLoader`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90229425
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/fs/hdfs/HadoopFileSystem.java ---
@@ -211,17 +209,14 @@ public HadoopFileSystem(Class<? extends org.apache.hadoop.fs.FileSystem> fsClass
if (new File(possibleHadoopConfPath).exists()) {
if (new File(possibleHadoopConfPath + "/core-site.xml").exists()) {
retConf.addResource(new org.apache.hadoop.fs.Path(possibleHadoopConfPath + "/core-site.xml"));
-
- if (LOG.isDebugEnabled()) {
- LOG.debug("Adding " + possibleHadoopConfPath + "/core-site.xml to hadoop configuration");
- }
+ } else {
+ LOG.debug("File " + possibleHadoopConfPath + "/core-site.xml not found.");
}
+
if (new File(possibleHadoopConfPath + "/hdfs-site.xml").exists()) {
retConf.addResource(new org.apache.hadoop.fs.Path(possibleHadoopConfPath + "/hdfs-site.xml"));
-
- if (LOG.isDebugEnabled()) {
- LOG.debug("Adding " + possibleHadoopConfPath + "/hdfs-site.xml to hadoop configuration");
- }
+ } else {
+ LOG.debug("File " + possibleHadoopConfPath + "/hdfs-site.xml not found.");
--- End diff --
Same here with `{}`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90229315
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/TaskState.java ---
@@ -168,4 +168,21 @@ public boolean equals(Object obj) {
public int hashCode() {
return parallelism + 31 * Objects.hash(jobVertexID, subtaskStates, kvStates);
}
+
+ @Override
+ public String toString() {
+ long stateSize = 0L;
+ for (SubtaskState subtaskState : subtaskStates.values()) {
+ stateSize += subtaskState.getStateSize();
+ }
--- End diff --
can't we use the `getStateSize` method here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2899: Various logging improvements
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2899
We should also set the received and handled message in `FlinkUntypedActor#onReceive` to trace.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2899: Various logging improvements
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/2899#discussion_r90233890
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/TaskState.java ---
@@ -168,4 +168,21 @@ public boolean equals(Object obj) {
public int hashCode() {
return parallelism + 31 * Objects.hash(jobVertexID, subtaskStates, kvStates);
}
+
+ @Override
+ public String toString() {
+ long stateSize = 0L;
+ for (SubtaskState subtaskState : subtaskStates.values()) {
+ stateSize += subtaskState.getStateSize();
+ }
--- End diff --
Yes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---