[GitHub] flink pull request #2899: Various logging improvements

[uce]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[uce]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[rmetzger]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[uce]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[uce]

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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[tillrohrmann]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[tillrohrmann]

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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[tillrohrmann]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[tillrohrmann]

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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2899: Various logging improvements

[uce]

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 
Date:   2016-11-29T15:00:02Z

[FLINK-5201] [logging] Log loaded config properties on INFO level

commit bb837cece0f0945ab05e71eda5c3a728a64b390b
Author: Ufuk Celebi 
Date:   2016-11-29T15:04:48Z

[FLINK-5196] [logging] Don't log InputChannelDeploymentDescriptor

commit 6a21e85fc7d99ba687753b4989bb25eb19aaa3af
Author: Ufuk Celebi 
Date:   2016-11-29T16:15:27Z

[FLINK-5194] [logging] Log heartbeats on TRACE level

commit 4a91b78e52c440d5c736789cea0f8d80968339f8
Author: Ufuk Celebi 
Date:   2016-11-29T15:15:30Z

[FLINK-5198] [logging] Improve TaskState toString

commit 5a5a78a15c2c3a0b3d82446b98c0c73c26cb4a4d
Author: Ufuk Celebi 
Date:   2016-11-29T15:35:14Z

[FLINK-5199] [logging] Improve logging in ZooKeeperSubmittedJobGraphStore

commit 83ef198008e4dd84a4876b7a8cefc1f870ebe0a2
Author: Ufuk Celebi 
Date:   2016-11-29T16:08:53Z

[FLINK-5207] [logging] Decrease HadoopFileSystem logging

commit 98c63af63036178e9c8423b99360f42d18b2e388
Author: Ufuk Celebi 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---