-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55591/
-----------------------------------------------------------

Review request for geode, Bruce Schuchardt, Darrel Schneider, Jinmei Liao, 
Jared Stewart, and Kevin Duling.


Bugs: GEODE-2297
    https://issues.apache.org/jira/browse/GEODE-2297


Repository: geode


Description
-------

GEODE-2297: passively discover DistributedSystem instance in AlertAppender to 
use in append

AlertAppender was actively hitting a synchronization and fetching 
InternalDistributedSystem singleton in every append(!) call. This can produce a 
deadlock if an alert is generated during startup. 

This change removes the active fetch from the append code path and uses 
InternalDistributedSystem.ConnectListener and 
InternalDistributedSystem.DisconnectListener to receive notifications of 
connect and disconnect. These notifications set an AtomicReference that is now 
checked within append. (Registering a ConnectListener is static and remains 
registered for the life of the JVM. For every onConnect, the AlertAppender 
registers a non-static DisconnectListener for each InternalDistributedSystem 
instance.)

I also fixed some minor IDE warnings, changed a logger statement from info to 
debug and added a TODO to the class' logger instance. I want to follow up later 
to *probably* change from using Logger to StatusLogger. Note: we run with 
StatusLogger level of FATAL by default. Appender impl's really should use 
StatusLogger instead of Logger (to avoid risk of infinite looping). As it's 
currently written (with Logger), you'd have to set the AlertLevel to DEBUG to 
hit this problem. I believe someone would only do this by error or for 
experimentation.


Diffs
-----

  
geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java
 a6c54aba352ba4dd3ece94c94f770292db6e9284 

Diff: https://reviews.apache.org/r/55591/diff/


Testing
-------

precheckin in progress


Thanks,

Kirk Lund

Reply via email to