accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 20465: ACCUMULO-2343 - AsyncSocketAppender
Date Tue, 22 Apr 2014 13:11:01 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message