> On April 21, 2014, 5:08 p.m., Josh Elser wrote: > > Did you also verify that you can run an instance using only the > > log4j.properties? Perhaps wrapping back around to earlier comments, have > > you also verified that updates to log4j.properties reload the logging > > backend? I'm not sure if log4j configures and watches log4j.properties by > > default. Also, what happens if I have both *_logger.xml files and > > log4j.properties files on the classpath which conflict each other (one sets > > some logger to WARN and the other to DEBUG) -- which one will actually be > > set?
I didn't perform those higher-order tests at this time because ACCUMULO-2383 covers integration testing of this appender with the rest of the system. This is a good list of things to ensure, though. >From looking forward, I know that the Accumulo and MonitorLog4jWatcher classes >will need to be updated to use either a Log4j DOMConfigurator or >PropertyConfigurator, depending on the type of configuration file. >PropertyConfigurator can set a watch on a file. The Accumulo class specifically looks for *_logger.xml files now, so we should be free to prioritize either properties or XML files in that logic. (If it were left to Log4j's default search order, then it would only look for log4j.properties.) > On April 21, 2014, 5:08 p.m., Josh Elser wrote: > > test/src/test/java/org/apache/accumulo/test/functional/MonitorLoggingIT.java, > > line 48 > > <https://reviews.apache.org/r/20465/diff/2/?file=562356#file562356line48> > > > > Starting the Monitor before the cluster starts mind eliminate some > > churn. I forget if the tserver just has a watcher set to see the update or > > if it polls periodically. I'll give that a go. > On April 21, 2014, 5:08 p.m., Josh Elser wrote: > > test/src/test/java/org/apache/accumulo/test/functional/MonitorLoggingIT.java, > > line 49 > > <https://reviews.apache.org/r/20465/diff/2/?file=562356#file562356line49> > > > > Would be better to actually look in zookeeper for when the monitor > > registers itself using a Watcher than just guessing that 10s is long enough. Quite true, I'll try that. > On April 21, 2014, 5:08 p.m., Josh Elser wrote: > > test/src/test/java/org/apache/accumulo/test/functional/MonitorLoggingIT.java, > > line 57 > > <https://reviews.apache.org/r/20465/diff/2/?file=562356#file562356line57> > > > > You can easily create a monitor entry by trying to scan a table with a > > SKVI of "java.lang.String" or something of the sort. It would be good to > > ensure that you generated a message before checking. I'll give that a shot. Definitely better than just assuming messages will appear. Thanks for all the quality feedback. :) - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20465/#review40942 ----------------------------------------------------------- On April 18, 2014, 3:14 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20465/ > ----------------------------------------------------------- > > (Updated April 18, 2014, 3:14 p.m.) > > > Review request for accumulo and Josh Elser. > > > Bugs: ACCUMULO-2343 > https://issues.apache.org/jira/browse/ACCUMULO-2343 > > > Repository: accumulo > > > Description > ------- > > AsyncSocketAppender is a Log4J AsyncAppender with its own internal > SocketAppender. Configuration for either appender can be set on the > AsyncSocketAppender itself. An AsyncSocketAppender can be configured using a > Log4J properties file, while an ordinary AsyncAppender cannot. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/util/AsyncSocketAppender.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/util/AsyncSocketAppenderTest.java > PRE-CREATION > > test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java > d9bed7f > > test/src/test/java/org/apache/accumulo/test/functional/MonitorLoggingIT.java > PRE-CREATION > test/src/test/resources/conf/generic_logger.xml PRE-CREATION > test/src/test/resources/conf/monitor_logger.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/20465/diff/ > > > Testing > ------- > > Unit test created and passed. Also used main() method to send log messages to > a running Log4J SimpleSocketServer instance. > > > Thanks, > > Bill Havanki > >
