[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

classic Classic list List threaded Threaded
53 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
GitHub user mikewalch opened a pull request:

    https://github.com/apache/accumulo/pull/221

    ACCUMULO-4588 Simplify Accumulo logging configuration

    * Removed log4j config being done in Java but some remains
    * Logging is now configured using standard log4j JVM property
      'log4.configuration' in accumulo-env.sh
    * Tarball ships with less log4j config files (3 rather than 6)
      which are all log4j properties files.
    * Log4j XML can still be used by editing accumulo-env.sh
    * Moved AsyncSocketAppend to start module due to classpath issues
    * Removed auditLog.xml and added audit log configuration to
      log4j-service & log4j-monitor properties files
    * Accumulo conf/ directory no longer has an examples/ directory.
      Configuration files ship in conf/ and are used by default.
    * Accumulo monitor by default will bind to 0.0.0.0 but will
      advertise hostname looked up in Java for log forwarding
    * Shortened names of logging system properties
    * Removed MonitorLoggingIT as it is now difficult to setup log
      forwarding using MiniAccumuloCluster.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mikewalch/accumulo accumulo-4588

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/221.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 #221
   
----

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102996101
 
    --- Diff: assemble/conf/log4j-monitor.properties ---
    @@ -36,6 +36,17 @@ log4j.appender.GUI=org.apache.accumulo.server.monitor.LogService
     log4j.appender.GUI.Keep=50
     log4j.appender.GUI.Threshold=WARN
     
    +## Configures Audit logs which are OFF by default.
    +#log4j.appender.AUDIT=org.apache.log4j.DailyRollingFileAppender
    +#log4j.appender.AUDIT.File=${accumulo.log.dir}/${accumulo.local.hostname}.audit
    +#log4j.appender.AUDIT.DatePattern='.'yyyy-MM-dd
    +#log4j.appender.AUDIT.layout=org.apache.log4j.PatternLayout
    +#log4j.appender.AUDIT.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss,SSS/Z} [%c{2}] %-5p: %m%n
    +#log4j.logger.org.apache.accumulo.audit=INFO, AUDIT
    +#log4j.additivity.org.apache.accumulo.audit=false
    +## Uncomment above and comment out line below to turn Audit logging ON
    +log4j.logger.org.apache.accumulo.audit=OFF
    --- End diff --
   
    Should this be included in log4j-monitor.properties? I would have only expected to see it in log4j-service.properties.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102995223
 
    --- Diff: assemble/conf/templates/accumulo-env.sh ---
    @@ -25,6 +25,21 @@
     # Variables that must be set
     ############################
     
    +## Accumulo logs directory. Referenced by logger config.
    +export ACCUMULO_LOG_DIR="${ACCUMULO_LOG_DIR:-$ACCUMULO_HOME/logs}"
    +## Hadoop installation
    +export HADOOP_PREFIX="${HADOOP_PREFIX:-/path/to/hadoop}"
    +## Hadoop configuration
    +export HADOOP_CONF_DIR="${HADOOP_CONF_DIR:-${HADOOP_PREFIX}/etc/hadoop}"
    +## Zookeeper installation
    +export ZOOKEEPER_HOME="${ZOOKEEPER_HOME:-/path/to/zookeeper}"
    +## See HADOOP-7154 and ACCUMULO-847
    +export MALLOC_ARENA_MAX=${MALLOC_ARENA_MAX:-1}
    --- End diff --
   
    Should this be under the "Variables that must be set" section? I wouldn't think the average user needs to set MALLOC_ARENA_MAX..


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102995712
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/Accumulo.java ---
    @@ -108,69 +105,20 @@ public static synchronized Path getAccumuloInstanceIdPath(VolumeManager fs) {
         return ServerConstants.getInstanceIdLocation(v);
       }
     
    -  /**
    -   * Finds the best log4j configuration file. A generic file is used only if an application-specific file is not available. An XML file is preferred over a
    -   * properties file, if possible.
    -   *
    -   * @param confDir
    -   *          directory where configuration files should reside
    -   * @param application
    -   *          application name for configuration file name
    -   * @return configuration file name
    -   */
    -  static String locateLogConfig(String confDir, String application) {
    -    String explicitConfigFile = System.getProperty("log4j.configuration");
    -    if (explicitConfigFile != null) {
    -      return explicitConfigFile;
    -    }
    -    String[] configFiles = {String.format("%s/%s_logger.xml", confDir, application), String.format("%s/%s_logger.properties", confDir, application),
    -        String.format("%s/examples/%s_logger.xml", confDir, application), String.format("%s/examples/%s_logger.properties", confDir, application),
    -        String.format("%s/generic_logger.xml", confDir), String.format("%s/generic_logger.properties", confDir),
    -        String.format("%s/examples/generic_logger.xml", confDir), String.format("%s/examples/generic_logger.properties", confDir),};
    -    String defaultConfigFile = String.format("%s/examples/generic_logger.xml", confDir);
    -    for (String f : configFiles) {
    -      if (new File(f).exists()) {
    -        return f;
    -      }
    -    }
    -    return defaultConfigFile;
    -  }
    -
    -  public static void setupLogging(String application) throws UnknownHostException {
    -    System.setProperty("org.apache.accumulo.core.application", application);
    -
    -    if (System.getenv("ACCUMULO_LOG_DIR") != null) {
    -      System.setProperty("org.apache.accumulo.core.dir.log", System.getenv("ACCUMULO_LOG_DIR"));
    -    }
    -
    -    String localhost = InetAddress.getLocalHost().getHostName();
    -    System.setProperty("org.apache.accumulo.core.ip.localhost.hostname", localhost);
    -
    -    // Use a specific log config, if it exists
    -    String logConfigFile = locateLogConfig(System.getenv("ACCUMULO_CONF_DIR"), application);
    -    // Turn off messages about not being able to reach the remote logger... we protect against that.
    -    LogLog.setQuietMode(true);
    -
    -    // Set up local file-based logging right away
    -    Log4jConfiguration logConf = new Log4jConfiguration(logConfigFile);
    -    logConf.resetLogger();
    -  }
    -
       public static void init(VolumeManager fs, ServerConfigurationFactory serverConfig, String application) throws IOException {
         final AccumuloConfiguration conf = serverConfig.getConfiguration();
         final Instance instance = serverConfig.getInstance();
     
    -    // Use a specific log config, if it exists
    -    final String logConfigFile = locateLogConfig(System.getenv("ACCUMULO_CONF_DIR"), application);
    -
    -    // Set up polling log4j updates and log-forwarding using information advertised in zookeeper by the monitor
    -    MonitorLog4jWatcher logConfigWatcher = new MonitorLog4jWatcher(instance.getInstanceID(), logConfigFile);
    -    logConfigWatcher.setDelay(5000L);
    -    logConfigWatcher.start();
    -
    -    // Makes sure the log-forwarding to the monitor is configured
    -    int[] logPort = conf.getPort(Property.MONITOR_LOG4J_PORT);
    -    System.setProperty("org.apache.accumulo.core.host.log.port", Integer.toString(logPort[0]));
    +    String logConfigFile = System.getProperty("log4j.configuration");
    +    if (logConfigFile != null) {
    +      if (logConfigFile.startsWith("file:")) {
    --- End diff --
   
    Do we have to parse this ourselves? Is there some class that would parse it for us?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102996636
 
    --- Diff: assemble/conf/templates/accumulo-env.sh ---
    @@ -33,24 +48,22 @@ JAVA_OPTS=("${ACCUMULO_JAVA_OPTS[@]}" '-XX:+UseConcMarkSweepGC' '-XX:CMSInitiati
     ## JVM options set for individual applications
     case "$ACCUMULO_CMD" in
     master)  JAVA_OPTS=("${JAVA_OPTS[@]}" ${masterHigh_masterLow}) ;;
    +monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     gc)      JAVA_OPTS=("${JAVA_OPTS[@]}" ${gcHigh_gcLow}) ;;
     tserver) JAVA_OPTS=("${JAVA_OPTS[@]}" ${tServerHigh_tServerLow}) ;;
    -monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     shell)   JAVA_OPTS=("${JAVA_OPTS[@]}" ${shellHigh_shellLow}) ;;
     *)       JAVA_OPTS=("${JAVA_OPTS[@]}" ${otherHigh_otherLow}) ;;
     esac
    -export JAVA_OPTS
     
    -## Accumulo logs directory. Referenced by logger config.
    -export ACCUMULO_LOG_DIR="${ACCUMULO_LOG_DIR:-$ACCUMULO_HOME/logs}"
    -## Hadoop installation
    -export HADOOP_PREFIX="${HADOOP_PREFIX:-/path/to/hadoop}"
    -## Hadoop configuration
    -export HADOOP_CONF_DIR="${HADOOP_CONF_DIR:-${HADOOP_PREFIX}/etc/hadoop}"
    -## Zookeeper installation
    -export ZOOKEEPER_HOME="${ZOOKEEPER_HOME:-/path/to/zookeeper}"
    -## See HADOOP-7154 and ACCUMULO-847
    -export MALLOC_ARENA_MAX=${MALLOC_ARENA_MAX:-1}
    +## JVM options set for logging
    +JAVA_OPTS=("${JAVA_OPTS[@]}" "-Daccumulo.application=${ACCUMULO_CMD}" "-Daccumulo.log.dir=${ACCUMULO_LOG_DIR}" "-Daccumulo.local.hostname=$(hostname -s)")
    --- End diff --
   
    I'm not 100% sure off the top of my head, but shouldn't this be `hostname -f` for consistency with what we had before? I like using the FQDN (the canonical name) to refer to nodes as a general rule.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102995985
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
     # Write out everything at the DEBUG level to the debug log
     log4j.appender.A2=org.apache.log4j.RollingFileAppender
    -log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
    -log4j.appender.A2.MaxFileSize=1000MB
    -log4j.appender.A2.MaxBackupIndex=10
    +log4j.appender.A2.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.debug.log
    +log4j.appender.A2.MaxFileSize=100MB
    +log4j.appender.A2.MaxBackupIndex=5
     log4j.appender.A2.Threshold=DEBUG
     log4j.appender.A2.layout=org.apache.log4j.PatternLayout
     log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
     
     # Write out INFO and higher to the regular log
     log4j.appender.A3=org.apache.log4j.RollingFileAppender
    -log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
    -log4j.appender.A3.MaxFileSize=1000MB
    -log4j.appender.A3.MaxBackupIndex=10
    +log4j.appender.A3.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.log
    +log4j.appender.A3.MaxFileSize=100MB
    +log4j.appender.A3.MaxBackupIndex=5
    --- End diff --
   
    Why change the default maxFileSize and number of backups?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102995483
 
    --- Diff: start/src/test/java/org/apache/accumulo/start/util/AsyncSocketAppenderTest.java ---
    @@ -36,7 +31,7 @@
     
       @Before
       public void setUp() throws Exception {
    -    sa = createMock(SocketAppender.class);
    +    sa = EasyMock.createMock(SocketAppender.class);
    --- End diff --
   
    Why make all of the changes in this file to remove the static imports? Seems unnecessary.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102999845
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
     # Write out everything at the DEBUG level to the debug log
     log4j.appender.A2=org.apache.log4j.RollingFileAppender
    -log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
    -log4j.appender.A2.MaxFileSize=1000MB
    -log4j.appender.A2.MaxBackupIndex=10
    +log4j.appender.A2.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.debug.log
    +log4j.appender.A2.MaxFileSize=100MB
    +log4j.appender.A2.MaxBackupIndex=5
     log4j.appender.A2.Threshold=DEBUG
     log4j.appender.A2.layout=org.apache.log4j.PatternLayout
     log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
     
     # Write out INFO and higher to the regular log
     log4j.appender.A3=org.apache.log4j.RollingFileAppender
    -log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
    -log4j.appender.A3.MaxFileSize=1000MB
    -log4j.appender.A3.MaxBackupIndex=10
    +log4j.appender.A3.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.log
    +log4j.appender.A3.MaxFileSize=100MB
    +log4j.appender.A3.MaxBackupIndex=5
    --- End diff --
   
    They were set to keep 20GB (10G for log & 10G for debug.log) of logs for each Accumulo service (tserver, master, gc, etc).  I understand that a user might want to do that on a beefy production node but I don't think it should be the default for users out of the box.  I have run into the problem of disks filling up due to logs when running a SSD VMs in AWS if these defaults are used.  This change will keep up to 1GB of logs per service on each node.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102989191
 
    --- Diff: assemble/conf/log4j-monitor.properties ---
    @@ -36,6 +36,17 @@ log4j.appender.GUI=org.apache.accumulo.server.monitor.LogService
     log4j.appender.GUI.Keep=50
     log4j.appender.GUI.Threshold=WARN
     
    +## Configures Audit logs which are OFF by default.
    --- End diff --
   
    Does the monitor need this?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r102989431
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
    --- End diff --
   
    Would it make sense to add some comments about this files intended use by Accumulo?
   
    Also, it may be helpful to mention that these are log4j 1.2.X config files, to help users find the appropriate documentation.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103000929
 
    --- Diff: assemble/conf/log4j-monitor.properties ---
    @@ -36,6 +36,17 @@ log4j.appender.GUI=org.apache.accumulo.server.monitor.LogService
     log4j.appender.GUI.Keep=50
     log4j.appender.GUI.Threshold=WARN
     
    +## Configures Audit logs which are OFF by default.
    +#log4j.appender.AUDIT=org.apache.log4j.DailyRollingFileAppender
    +#log4j.appender.AUDIT.File=${accumulo.log.dir}/${accumulo.local.hostname}.audit
    +#log4j.appender.AUDIT.DatePattern='.'yyyy-MM-dd
    +#log4j.appender.AUDIT.layout=org.apache.log4j.PatternLayout
    +#log4j.appender.AUDIT.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss,SSS/Z} [%c{2}] %-5p: %m%n
    +#log4j.logger.org.apache.accumulo.audit=INFO, AUDIT
    +#log4j.additivity.org.apache.accumulo.audit=false
    +## Uncomment above and comment out line below to turn Audit logging ON
    +log4j.logger.org.apache.accumulo.audit=OFF
    --- End diff --
   
    It looks like there is no auditing in the monitor so I will remove.  It can be added back if auditing is ever added.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103003156
 
    --- Diff: start/src/test/java/org/apache/accumulo/start/util/AsyncSocketAppenderTest.java ---
    @@ -36,7 +31,7 @@
     
       @Before
       public void setUp() throws Exception {
    -    sa = createMock(SocketAppender.class);
    +    sa = EasyMock.createMock(SocketAppender.class);
    --- End diff --
   
    My IDE was having problems with the static imports.  Not sure why.  I could have looked into it more but I am not a fan of static imports of methods like that as it makes the code harder to read.  Therefore, I removed them.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103003251
 
    --- Diff: assemble/conf/templates/accumulo-env.sh ---
    @@ -25,6 +25,21 @@
     # Variables that must be set
     ############################
     
    +## Accumulo logs directory. Referenced by logger config.
    +export ACCUMULO_LOG_DIR="${ACCUMULO_LOG_DIR:-$ACCUMULO_HOME/logs}"
    +## Hadoop installation
    +export HADOOP_PREFIX="${HADOOP_PREFIX:-/path/to/hadoop}"
    +## Hadoop configuration
    +export HADOOP_CONF_DIR="${HADOOP_CONF_DIR:-${HADOOP_PREFIX}/etc/hadoop}"
    +## Zookeeper installation
    +export ZOOKEEPER_HOME="${ZOOKEEPER_HOME:-/path/to/zookeeper}"
    +## See HADOOP-7154 and ACCUMULO-847
    +export MALLOC_ARENA_MAX=${MALLOC_ARENA_MAX:-1}
    --- End diff --
   
    I will fix


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103004960
 
    --- Diff: assemble/conf/templates/accumulo-env.sh ---
    @@ -33,24 +48,22 @@ JAVA_OPTS=("${ACCUMULO_JAVA_OPTS[@]}" '-XX:+UseConcMarkSweepGC' '-XX:CMSInitiati
     ## JVM options set for individual applications
     case "$ACCUMULO_CMD" in
     master)  JAVA_OPTS=("${JAVA_OPTS[@]}" ${masterHigh_masterLow}) ;;
    +monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     gc)      JAVA_OPTS=("${JAVA_OPTS[@]}" ${gcHigh_gcLow}) ;;
     tserver) JAVA_OPTS=("${JAVA_OPTS[@]}" ${tServerHigh_tServerLow}) ;;
    -monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     shell)   JAVA_OPTS=("${JAVA_OPTS[@]}" ${shellHigh_shellLow}) ;;
     *)       JAVA_OPTS=("${JAVA_OPTS[@]}" ${otherHigh_otherLow}) ;;
     esac
    -export JAVA_OPTS
     
    -## Accumulo logs directory. Referenced by logger config.
    -export ACCUMULO_LOG_DIR="${ACCUMULO_LOG_DIR:-$ACCUMULO_HOME/logs}"
    -## Hadoop installation
    -export HADOOP_PREFIX="${HADOOP_PREFIX:-/path/to/hadoop}"
    -## Hadoop configuration
    -export HADOOP_CONF_DIR="${HADOOP_CONF_DIR:-${HADOOP_PREFIX}/etc/hadoop}"
    -## Zookeeper installation
    -export ZOOKEEPER_HOME="${ZOOKEEPER_HOME:-/path/to/zookeeper}"
    -## See HADOOP-7154 and ACCUMULO-847
    -export MALLOC_ARENA_MAX=${MALLOC_ARENA_MAX:-1}
    +## JVM options set for logging
    +JAVA_OPTS=("${JAVA_OPTS[@]}" "-Daccumulo.application=${ACCUMULO_CMD}" "-Daccumulo.log.dir=${ACCUMULO_LOG_DIR}" "-Daccumulo.local.hostname=$(hostname -s)")
    --- End diff --
   
    The 'accumulo.local.hostname' property is only used in the log4j properties files to set the hostname in file names for logs and to identify the host when logs are forwarded to the monitor.  I prefer the shorter hostname for these two cases but it's not a big issue for me and I will switch to `hostname -f` if you prefer.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103006755
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
     # Write out everything at the DEBUG level to the debug log
     log4j.appender.A2=org.apache.log4j.RollingFileAppender
    -log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
    -log4j.appender.A2.MaxFileSize=1000MB
    -log4j.appender.A2.MaxBackupIndex=10
    +log4j.appender.A2.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.debug.log
    +log4j.appender.A2.MaxFileSize=100MB
    +log4j.appender.A2.MaxBackupIndex=5
     log4j.appender.A2.Threshold=DEBUG
     log4j.appender.A2.layout=org.apache.log4j.PatternLayout
     log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
     
     # Write out INFO and higher to the regular log
     log4j.appender.A3=org.apache.log4j.RollingFileAppender
    -log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
    -log4j.appender.A3.MaxFileSize=1000MB
    -log4j.appender.A3.MaxBackupIndex=10
    +log4j.appender.A3.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.log
    +log4j.appender.A3.MaxFileSize=100MB
    +log4j.appender.A3.MaxBackupIndex=5
    --- End diff --
   
    I think the reduction in MaxFileSize is OK but only having 5 for MaxBackupIndex files seems small.  That isn't even a full week.  You could keep it at 10 since you are already reducing the size.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103007628
 
    --- Diff: assemble/conf/templates/accumulo-env.sh ---
    @@ -33,24 +48,22 @@ JAVA_OPTS=("${ACCUMULO_JAVA_OPTS[@]}" '-XX:+UseConcMarkSweepGC' '-XX:CMSInitiati
     ## JVM options set for individual applications
     case "$ACCUMULO_CMD" in
     master)  JAVA_OPTS=("${JAVA_OPTS[@]}" ${masterHigh_masterLow}) ;;
    +monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     gc)      JAVA_OPTS=("${JAVA_OPTS[@]}" ${gcHigh_gcLow}) ;;
     tserver) JAVA_OPTS=("${JAVA_OPTS[@]}" ${tServerHigh_tServerLow}) ;;
    -monitor) JAVA_OPTS=("${JAVA_OPTS[@]}" ${monitorHigh_monitorLow}) ;;
     shell)   JAVA_OPTS=("${JAVA_OPTS[@]}" ${shellHigh_shellLow}) ;;
     *)       JAVA_OPTS=("${JAVA_OPTS[@]}" ${otherHigh_otherLow}) ;;
     esac
    -export JAVA_OPTS
     
    -## Accumulo logs directory. Referenced by logger config.
    -export ACCUMULO_LOG_DIR="${ACCUMULO_LOG_DIR:-$ACCUMULO_HOME/logs}"
    -## Hadoop installation
    -export HADOOP_PREFIX="${HADOOP_PREFIX:-/path/to/hadoop}"
    -## Hadoop configuration
    -export HADOOP_CONF_DIR="${HADOOP_CONF_DIR:-${HADOOP_PREFIX}/etc/hadoop}"
    -## Zookeeper installation
    -export ZOOKEEPER_HOME="${ZOOKEEPER_HOME:-/path/to/zookeeper}"
    -## See HADOOP-7154 and ACCUMULO-847
    -export MALLOC_ARENA_MAX=${MALLOC_ARENA_MAX:-1}
    +## JVM options set for logging
    +JAVA_OPTS=("${JAVA_OPTS[@]}" "-Daccumulo.application=${ACCUMULO_CMD}" "-Daccumulo.log.dir=${ACCUMULO_LOG_DIR}" "-Daccumulo.local.hostname=$(hostname -s)")
    --- End diff --
   
    Ah, I could be swayed either way in that regard. Looking at a local install I have laying around, the FQDN is presently used:
   
    `master_hw10447.local.debug.log`, `master_hw10447.local.log`
   
    Consistency would probably be best.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103007972
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
     # Write out everything at the DEBUG level to the debug log
     log4j.appender.A2=org.apache.log4j.RollingFileAppender
    -log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
    -log4j.appender.A2.MaxFileSize=1000MB
    -log4j.appender.A2.MaxBackupIndex=10
    +log4j.appender.A2.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.debug.log
    +log4j.appender.A2.MaxFileSize=100MB
    +log4j.appender.A2.MaxBackupIndex=5
     log4j.appender.A2.Threshold=DEBUG
     log4j.appender.A2.layout=org.apache.log4j.PatternLayout
     log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
     
     # Write out INFO and higher to the regular log
     log4j.appender.A3=org.apache.log4j.RollingFileAppender
    -log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
    -log4j.appender.A3.MaxFileSize=1000MB
    -log4j.appender.A3.MaxBackupIndex=10
    +log4j.appender.A3.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.log
    +log4j.appender.A3.MaxFileSize=100MB
    +log4j.appender.A3.MaxBackupIndex=5
    --- End diff --
   
    Ok, sounds good. Just wanted to call out the change since it wasn't included in the commit msg.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103009716
 
    --- Diff: docs/src/main/asciidoc/chapters/administration.txt ---
    @@ -1132,12 +1130,12 @@ can be exacerbated by resource constraints and clock drift.
     ==== Tested Versions
     Each release of Accumulo is built with a specific version of Apache
     Hadoop, Apache ZooKeeper and Apache Thrift.  We expect Accumulo to
    -work with versions that are API compatable with those versions.
    +work with versions that are API compatible with those versions.
     However this compatibility is not guaranteed because Hadoop, ZooKeeper
     and Thift may not provide guarantees between their own versions. We
     have also found that certain versions of Accumulo and Hadoop included
     bugs that greatly affected overall stability.  Thrift is particularly
    -prone to compatablity changes between versions and you must use the
    +prone to compatibility changes between versions and you must use the
    --- End diff --
   
    A+ on spelling


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103011353
 
    --- Diff: assemble/conf/log4j-service.properties ---
    @@ -15,42 +15,48 @@
     
     # Write out everything at the DEBUG level to the debug log
     log4j.appender.A2=org.apache.log4j.RollingFileAppender
    -log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
    -log4j.appender.A2.MaxFileSize=1000MB
    -log4j.appender.A2.MaxBackupIndex=10
    +log4j.appender.A2.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.debug.log
    +log4j.appender.A2.MaxFileSize=100MB
    +log4j.appender.A2.MaxBackupIndex=5
     log4j.appender.A2.Threshold=DEBUG
     log4j.appender.A2.layout=org.apache.log4j.PatternLayout
     log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
     
     # Write out INFO and higher to the regular log
     log4j.appender.A3=org.apache.log4j.RollingFileAppender
    -log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
    -log4j.appender.A3.MaxFileSize=1000MB
    -log4j.appender.A3.MaxBackupIndex=10
    +log4j.appender.A3.File=${accumulo.log.dir}/${accumulo.application}${accumulo.service.instance}_${accumulo.local.hostname}.log
    +log4j.appender.A3.MaxFileSize=100MB
    +log4j.appender.A3.MaxBackupIndex=5
    --- End diff --
   
    @milleruntime, I change MaxBackupIndex back to 10 but RollingFileAppender rolls when MaxFileSize is reached.  There is a DailyRollingFileAppender used for audit log that rolls daily.  Logs will be rolled off by size rather than time so it depends on how much you are sending to logs.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #221: ACCUMULO-4588 Simplify Accumulo logging configur...

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/221#discussion_r103011409
 
    --- Diff: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java ---
    @@ -496,20 +497,27 @@ public void run(String hostname) {
           throw new RuntimeException(e);
         }
     
    -    try {
    -      log.debug("Using " + hostname + " to advertise monitor location in ZooKeeper");
    -
    -      String monitorAddress = HostAndPort.fromParts(hostname, server.getPort()).toString();
    +    String advertiseHost = hostname;
    +    if (advertiseHost.equals("0.0.0.0")) {
    +      try {
    +        advertiseHost = InetAddress.getLocalHost().getHostName();
    +      } catch (UnknownHostException e) {
    +        log.error("Unable to get hostname", e);
    +      }
    +    }
    +    log.debug("Using " + advertiseHost + " to advertise monitor location");
     
    +    try {
    +      String monitorAddress = HostAndPort.fromParts(advertiseHost, server.getPort()).toString();
           ZooReaderWriter.getInstance().putPersistentData(ZooUtil.getRoot(instance) + Constants.ZMONITOR_HTTP_ADDR, monitorAddress.getBytes(UTF_8),
               NodeExistsPolicy.OVERWRITE);
           log.info("Set monitor address in zookeeper to " + monitorAddress);
         } catch (Exception ex) {
           log.error("Unable to set monitor HTTP address in zookeeper", ex);
         }
     
    -    if (null != hostname) {
    -      LogService.startLogListener(Monitor.getContext().getConfiguration(), instance.getInstanceID(), hostname);
    +    if (null != advertiseHost) {
    +      LogService.startLogListener(Monitor.getContext().getConfiguration(), instance.getInstanceID(), advertiseHost);
    --- End diff --
   
    Wouldn't hurt to sync up with Luis on changes to the Monitor


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
123