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