Skip to content

Conversation

Dreeam-qwq
Copy link

@Dreeam-qwq Dreeam-qwq commented Oct 11, 2023

Add support for disruptor 4.0.0 to close LOG4J2-3595 (#1829). Tested and looks good.

@ppkarwasz
Copy link
Contributor

ppkarwasz commented Oct 11, 2023

@Dreeam-qwq,

All 2.x releases must run on JDK 8. Since Disruptor 4.x requires a Java 11 runtime, we can not drop support for Disruptor 3.x.

Looking at your patch, the only breaking change that occurred is the disappearance of the SequenceReportingEventHandler and LifecycleAware interfaces, so you can work around these restrictions by:

  • restoring version 3.4.4 in the POM file,
  • removing the @Override annotations from the new EventHandler methods (so that the code compiles with Disruptor 3.x),
  • create a RingBufferLogEventHandler3 and AsyncLoggerConfigDisruptor#Log4jEventWrapperHandler3 classes that extend their v4 counterparts and differ just in the implements clause:
    public class RingBufferLogEventHandler3 extends RingBufferLogEventHandler
        implements SequenceReportingEventHandler<RingBufferLogEvent>, LifecycleAware  {}
  • add logic to switch between RingBufferLogEventHandler3 and RingBufferLogEventHandler based on the existence of the SequenceReportingEventHandler interface (we have LoaderUtil#isClassAvailable to check for optional classes),
  • add some logic to switch between our TimeoutBlockingWaitStrategy and the one shipped with Disruptor 4.x.

To detect if we have Disruptor 3.x, you can call Loader.isClassAvailable(SequenceReportingEventHander.class).

To test the PR run:

./mvnw install -pl log4j-core

followed by:

./mvnw verify -Pjava8-tests -Dtest="org/apache/logging/log4j/core/async/*"

to test on JDK 8 (you need a Maven toolchains configuration) and Disruptor 3.x and

 ./mvnw verify -Dtest="org/apache/logging/log4j/core/async/*" -Ddisruptor.version=4.0.0

to test on JDK 11 and Disruptor 4.x.

@ppkarwasz
Copy link
Contributor

Closes #1829 .

@Dreeam-qwq
Copy link
Author

I'm sorry that I have no enough time to do this recently, I'll close this pr, sorry to waste your time to review it, also thanks for you to give explanation to me.

@Dreeam-qwq Dreeam-qwq closed this Oct 11, 2023
@ppkarwasz
Copy link
Contributor

@Dreeam-qwq,

Take your time. We won't be able to include this PR into 2.21.0 anyway, since the release process is already in motion.

You can also resubmit this PR against the main branch, which is based on Java 11, so most of the problems disappear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants