Skip to content

Commit

Permalink
Fix problems from apache#2112 reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
ppkarwasz committed Dec 20, 2023
1 parent 5068a03 commit 38e162a
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ public String toString() {

/**
* EventHandler performs the work in a separate thread.
* <p>
* <strong>Warning:</strong> this implementation only works with Disruptor 4.x.
* </p>
*/
private static class Log4jEventWrapperHandler implements EventHandler<Log4jEventWrapper> {
private static final int NOTIFY_PROGRESS_THRESHOLD = 50;
Expand Down Expand Up @@ -129,7 +132,10 @@ private void notifyIntermediateProgress(final long sequence) {
}

/**
* A version of Log4jEventWrapperHandler for LMAX Disruptor 3.x.
* EventHandler performs the work in a separate thread.
* <p>
* <strong>Warning:</strong> this implementation only works with Disruptor 3.x.
* </p>
*/
private static final class Log4jEventWrapperHandler3 extends Log4jEventWrapperHandler
implements SequenceReportingEventHandler<Log4jEventWrapper> {}
Expand Down Expand Up @@ -166,11 +172,13 @@ private static final class Log4jEventWrapperHandler3 extends Log4jEventWrapperHa
};

private Log4jEventWrapperHandler createEventHandler() {
try {
return LoaderUtil.newInstanceOf(
"org.apache.logging.log4j.core.async.AsyncLoggerConfigDisruptor$Log4jEventWrapperHandler3");
} catch (final ReflectiveOperationException | LinkageError e) {
LOGGER.debug("LMAX Disruptor 3.x is missing, trying version 4.x.", e);
if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 3) {
try {
return LoaderUtil.newInstanceOf(
"org.apache.logging.log4j.core.async.AsyncLoggerConfigDisruptor$Log4jEventWrapperHandler3");
} catch (final ReflectiveOperationException | LinkageError e) {
LOGGER.warn("Failed to create event handler for LMAX Disruptor 3.x, trying version 4.x.", e);
}
}
return new Log4jEventWrapperHandler();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.async;

import com.lmax.disruptor.EventHandler;
import com.lmax.disruptor.EventTranslatorVararg;
import com.lmax.disruptor.ExceptionHandler;
import com.lmax.disruptor.RingBuffer;
Expand All @@ -35,6 +36,7 @@
import org.apache.logging.log4j.core.util.Log4jThreadFactory;
import org.apache.logging.log4j.core.util.Throwables;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.util.LoaderUtil;

/**
* Helper class for async loggers: AsyncLoggerDisruptor handles the mechanics of working with the LMAX Disruptor, and
Expand All @@ -46,6 +48,20 @@ class AsyncLoggerDisruptor extends AbstractLifeCycle {
private static final int SLEEP_MILLIS_BETWEEN_DRAIN_ATTEMPTS = 50;
private static final int MAX_DRAIN_ATTEMPTS_BEFORE_SHUTDOWN = 200;

/**
* Creates an appropriate event handler for the Disruptor library used.
*/
private static EventHandler<RingBufferLogEvent> createEventHandler() {
if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 3) {
try {
return LoaderUtil.newInstanceOf("org.apache.logging.log4j.core.async.RingBufferLogEventHandler");
} catch (final ReflectiveOperationException | LinkageError e) {
LOGGER.warn("Failed to create event handler for LMAX Disruptor 3.x, trying version 4.x.", e);
}
}
return new RingBufferLogEventHandler4();
}

private final Object queueFullEnqueueLock = new Object();

private volatile Disruptor<RingBufferLogEvent> disruptor;
Expand Down Expand Up @@ -122,8 +138,8 @@ public Thread newThread(final Runnable r) {
final ExceptionHandler<RingBufferLogEvent> errorHandler = DisruptorUtil.getAsyncLoggerExceptionHandler();
disruptor.setDefaultExceptionHandler(errorHandler);

final RingBufferLogEventHandler4[] handlers = {RingBufferLogEventHandler4.create()};
disruptor.handleEventsWith(handlers);
final EventHandler<RingBufferLogEvent> handler = createEventHandler();
disruptor.handleEventsWith(handler);

LOGGER.debug(
"[{}] Starting AsyncLogger disruptor for this context with ringbufferSize={}, waitStrategy={}, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,17 @@ static WaitStrategy createDefaultWaitStrategy(final String propertyName) {
LOGGER.trace(
"DefaultAsyncWaitStrategyFactory creating TimeoutBlockingWaitStrategy(timeout={}, unit=MILLIS)",
timeoutMillis);
try {
// Check for the v 4.x version of the strategy, the version in 3.x is not garbage-free.
if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 4) {
// Check for the v 4.x version of the strategy, the version in 3.x is not garbage-free.
if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 4) {
try {
return (WaitStrategy) Class.forName("com.lmax.disruptor.TimeoutBlockingWaitStrategy")
.getConstructor(long.class, TimeUnit.class)
.newInstance(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (final ReflectiveOperationException | LinkageError e) {
LOGGER.debug(
"DefaultAsyncWaitStrategyFactory failed to load 'com.lmax.disruptor.TimeoutBlockingWaitStrategy', using '{}' instead.",
TimeoutBlockingWaitStrategy.class.getName());
}
} catch (final ReflectiveOperationException | LinkageError e) {
LOGGER.debug(
"DefaultAsyncWaitStrategyFactory failed to load 'com.lmax.disruptor.TimeoutBlockingWaitStrategy', using '{}' instead.",
TimeoutBlockingWaitStrategy.class.getName());
}
// Use our version
return new TimeoutBlockingWaitStrategy(timeoutMillis, TimeUnit.MILLISECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@
* available. Processing of these messages is done in a separate thread,
* controlled by the {@code Executor} passed to the {@code Disruptor}
* constructor.
* <p>
* <strong>Warning:</strong> this class only works with Disruptor 3.x.
* </p>
* @deprecated Only used internally, will be removed in the next major version.
*/
@Deprecated
public class RingBufferLogEventHandler extends RingBufferLogEventHandler4
implements SequenceReportingEventHandler<RingBufferLogEvent>, LifecycleAware {

/**
* @deprecated Use the {@link RingBufferLogEventHandler4#create()} factory method instead.
*/
@Deprecated
public RingBufferLogEventHandler() {}
}
implements SequenceReportingEventHandler<RingBufferLogEvent>, LifecycleAware {}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@

import com.lmax.disruptor.EventHandler;
import com.lmax.disruptor.Sequence;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.LoaderUtil;

/**
* This event handler gets passed messages from the RingBuffer as they become
* available. Processing of these messages is done in a separate thread,
* controlled by the {@code Executor} passed to the {@code Disruptor}
* constructor.
* * <p>
* * <strong>Warning:</strong> this class only works with Disruptor 4.x.
* * </p>
*/
class RingBufferLogEventHandler4 implements EventHandler<RingBufferLogEvent> {

Expand All @@ -34,18 +35,6 @@ class RingBufferLogEventHandler4 implements EventHandler<RingBufferLogEvent> {
private int counter;
private long threadId = -1;

/**
* Returns the appropriate {@link EventHandler} for the version of LMAX Disruptor used.
*/
public static RingBufferLogEventHandler4 create() {
try {
return LoaderUtil.newInstanceOf("org.apache.logging.log4j.core.async.RingBufferLogEventHandler");
} catch (final ReflectiveOperationException | LinkageError e) {
StatusLogger.getLogger().debug("LMAX Disruptor 3.x is missing, trying version 4.x.", e);
}
return new RingBufferLogEventHandler4();
}

/*
* Overrides a method from Disruptor 4.x. Do not remove.
*/
Expand Down

0 comments on commit 38e162a

Please sign in to comment.