You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2017/04/17 22:37:35 UTC
[GitHub] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
GitHub user selvaganesang opened a pull request:
https://github.com/apache/incubator-trafodion/pull/1068
[TRAFODION-2594] Trafodion logs the same message twice in most of its\u2026
\u2026 log4j and log4cxx logs
[TRAFODION-2420] RMS enhancements
[TRAFODION-2478] Memory monitor Thread enhancements
a) Changed the configuration to have log4j.additive property set to false
b) ScratchIOSize is initialized so that this counter doesn't show invalid
value
c) Cleaned up memory monitor concept and added code to log the memory monitoring
counters to sccp log file every 5 minutes or when there is a pressure difference
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/selvaganesang/incubator-trafodion trafodion-2420_2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/1068.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 #1068
----
commit a4df3302f5fdcbf8abd165fed8b0da48c2de132f
Author: selvaganesang <se...@esgyn.com>
Date: 2017-04-15T18:53:34Z
[TRAFODION-2594] Trafodion logs the same message twice in most of its log4j and log4cxx logs
[TRAFODION-2420] RMS enhancements
[TRAFODION-2478] Memory monitor Thread enhancements
a) Changed the configuration to have log4j.additive property set to false
b) ScratchIOSize is initialized so that this counter doesn't show invalid
value
c) Cleaned up memory monitor concept and added code to log the memory monitoring
counters to sccp log file every 5 minutes or when there is a pressure difference
----
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112101144
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
SQL doesn't log much in INFO level in C++ part of the code. Also, INFO level is preferred level. I
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112238169
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -43,19 +38,10 @@ log4j.appender.mxoAppender.addPid=false
log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
+log4j.additive.mxoAppender=false
+
#MXOSRVR
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
-# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
-log4j.logger.SQL=INFO
-log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
-log4j.logger.SQL.PRIVMGR=INFO
-#
-log4j.logger.SQL.HDFS.JniTop=FATAL
-log4j.logger.SQL.HDFS.SeqFileReader=FATAL
-log4j.logger.SQL.HDFS.SeqFileWriter=FATAL
-log4j.logger.SQL.HBase=FATAL
+log4j.logger.SQL=INFO, mxoAppender
--- End diff --
Yes. We will change these sqmxevents to DEBUG or TRACE level as and when we come across messages that are becoming dominating in the logs. INFO will be the production level setting.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112101626
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
Removed log4j.logger.SQL.COMP=ERROR in the 2nd commit. But kept ERROR level for MXOSRVR for now because MXOSRVR seems to log INFO level messages quite enthusiastically.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112087182
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
--- End diff --
Should this comment be removed? (Since the line it refers to was removed.)
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r111993000
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
--- End diff --
This will by default log all the informational events . In most cases we don't want those and normally turn them on by default only when needed.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112299127
--- Diff: core/sqf/conf/log4cxx.monitor.mon.config ---
@@ -22,17 +22,16 @@
#
# Define some default values that can be overridden by system properties
-trafodion.root.logger=INFO, monAppender
trafodion.log.dir=${TRAF_HOME}/logs
trafodion.log.filename.suffix=${TRAFODION_LOG_FILENAME_SUFFIX}
-# Define the root logger to the system property "trafodion.root.logger".
-log4j.rootLogger=${trafodion.root.logger}
-
# Logging Threshold
log4j.threshhold=ALL
-#
+# Define the root logger to the system property "trafodion.root.logger".
+trafodion.root.logger=FATAL
--- End diff --
I found another way to configure rootLogger, but at the same time not causing duplicate messages to be written. Changed the configuration files accordingly in commit 3
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r111993077
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
Same comment as above.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112096641
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
Ok . But we are definitely changing behaviour already. There is a generic method SQLMXLoggingArea::logExecRTInfo that uses INFO as the level. Am guessing it could affect performance since it's used in BMO code paths. Not sure we wnt the user to see all these messages by default. If you do a quick grep on our code you can see what is being logged using this generic method.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/1068
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112237344
--- Diff: core/sqf/conf/log4cxx.monitor.mon.config ---
@@ -22,17 +22,16 @@
#
# Define some default values that can be overridden by system properties
-trafodion.root.logger=INFO, monAppender
trafodion.log.dir=${TRAF_HOME}/logs
trafodion.log.filename.suffix=${TRAFODION_LOG_FILENAME_SUFFIX}
-# Define the root logger to the system property "trafodion.root.logger".
-log4j.rootLogger=${trafodion.root.logger}
-
# Logging Threshold
log4j.threshhold=ALL
-#
+# Define the root logger to the system property "trafodion.root.logger".
+trafodion.root.logger=FATAL
--- End diff --
I tried and the process didn't come up as I explained in my comments. Web searching revealed that it is ok not to configure rootLogger without appender though there is no default rootLogger appender
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112235074
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -43,19 +38,10 @@ log4j.appender.mxoAppender.addPid=false
log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
+log4j.additive.mxoAppender=false
+
#MXOSRVR
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
-# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
-log4j.logger.SQL=INFO
-log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
-log4j.logger.SQL.PRIVMGR=INFO
-#
-log4j.logger.SQL.HDFS.JniTop=FATAL
-log4j.logger.SQL.HDFS.SeqFileReader=FATAL
-log4j.logger.SQL.HDFS.SeqFileWriter=FATAL
-log4j.logger.SQL.HBase=FATAL
+log4j.logger.SQL=INFO, mxoAppender
--- End diff --
How come the granularity has changed ? Sometimes we want to log only Hbase or Hdfs messages. Having the different levels helps. Plus if we have to make the default INFO our code needs to change to not use INFO in sqlmxevents APIs to log debugging type of messages which it does today. Those are not meant for the end user.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112234144
--- Diff: core/sqf/conf/log4cxx.monitor.mon.config ---
@@ -22,17 +22,16 @@
#
# Define some default values that can be overridden by system properties
-trafodion.root.logger=INFO, monAppender
trafodion.log.dir=${TRAF_HOME}/logs
trafodion.log.filename.suffix=${TRAFODION_LOG_FILENAME_SUFFIX}
-# Define the root logger to the system property "trafodion.root.logger".
-log4j.rootLogger=${trafodion.root.logger}
-
# Logging Threshold
log4j.threshhold=ALL
-#
+# Define the root logger to the system property "trafodion.root.logger".
+trafodion.root.logger=FATAL
--- End diff --
Can you make the output atleats go to stdout ? The hierarchy of logging is supposed to be setup this way.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by venkat1m <gi...@git.apache.org>.
Github user venkat1m commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112098654
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
I'm not familiar with this code path. But generally we want to avoid flood of INFO messages. It makes debugging difficult when we have to scan through the logs for real problems.
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112094397
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
The hiearchy of the logging level is TRACE, DEBUG, INFO,WARN,ERROR and FATAL, It is good to have the logging level set to INFO to ensure that the users can decipher the problem without the need to prompte the logging level to the higher hierarchy. When a message is logged at the incorrect level, it can be corrected later
---
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] incubator-trafodion pull request #1068: [TRAFODION-2594] Trafodion logs the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1068#discussion_r112096781
--- Diff: core/sqf/conf/log4cxx.trafodion.masterexe.config ---
@@ -44,15 +45,15 @@ log4j.appender.mxoAppender.layout=org.apache.log4j.PatternLayout
log4j.appender.mxoAppender.layout.ConversionPattern=%d, %p, %c, %m%n
#MXOSRVR
+log4j.additive.mxoAppender=false
log4j.logger.MXOSRVR=ERROR, mxoAppender
# SQL
# uncomment the following line if you want to enable debug logging
-#log4j.logger.SQL.EXE=DEBUG
log4j.logger.SQL=INFO
log4j.logger.SQL.COMP=ERROR
-log4j.logger.SQL.EXE.Memory=INFO
-log4j.logger.SQL.ESP=ERROR
+log4j.logger.SQL.EXE=INFO
+log4j.logger.SQL.ESP=INFO
--- End diff --
Also in the same block compiler messages are logged under ERROR instead of INFO ? log4j.logger.SQL.COMP=ERROR
---
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.
---