Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Artemis 3163 #15

Open
wants to merge 3 commits into
base: io_uring_incubator
Choose a base branch
from
+65 −33
Open

Artemis 3163 #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97dbfc5
ARTEMIS-3163 - Fix logging declared order so its asc
michaelandrepearce Sep 28, 2021
b59723a
ARTEMIS-3163 Remove reflection
michaelandrepearce Sep 28, 2021
e008a40
ARTEMIS-3163 Add Documentation for IO URING
michaelandrepearce Sep 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions artemis-core-client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@
<artifactId>netty-transport-native-kqueue</artifactId>
<classifier>${netty-transport-native-kqueue-classifier}</classifier>
</dependency>
<dependency>
<groupId>io.netty.incubator</groupId>
<artifactId>netty-incubator-transport-native-io_uring</artifactId>
<classifier>${netty-transport-native-io_uring-classifier}</classifier>
</dependency>
Comment on lines +107 to +111
Copy link

@gemmellr gemmellr Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be optional for now until closer to the point its considered non experimental and has more than zero testing. I expect few to use it initially, and those who do can easily add it themselves, so it seems premature to make it a hard dependency for everyone.

Copy link
Author

@michaelandrepearce michaelandrepearce Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst i agree to testing. I disagree with not including the jar. Its behind a feature toggle, by not including it youre making it much tricker for users to test/try and give feedback on. Even myself whilst i can package and deploy to lower envs the apache dist for better real testing, and getting feedback i have to jump through alot of hoops to the point it makes it prohibitive to do that and get that feedback loop if i have to customise by adding jars outside distros

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would imagine it to be the same with softwaremill who run perf tests where they will just take the distro only. Which is one of the main targets we would be asking them to retest with this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont consider that an issue at this stage. I think its pretty trivial to customise things like this, little different to getting the distribution if thats what you do (many dont). Certainly not a good reason to include it, for me.

<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-codec-http</artifactId>
Expand Down
20 changes: 10 additions & 10 deletions ...re-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,6 @@ public interface ActiveMQClientLogger extends BasicLogger {
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckEpollAvailability(@Cause Throwable e);

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212080, value = "Unable to check IoUring availability ",
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckIoUringAvailability(@Cause Throwable e);

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212072, value = "Failed to change channel state to ReadyForWriting ",
format = Message.Format.MESSAGE_FORMAT)
Expand All @@ -425,11 +420,6 @@ public interface ActiveMQClientLogger extends BasicLogger {
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckEpollAvailabilitynoClass();

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212079, value = "IoUring is not available, please add to the classpath or configure useIoUring=false to remove this warning",
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckIoUringAvailabilitynoClass();

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212077, value = "Timed out waiting to receive initial broadcast from cluster. Retry {0} of {1}",
format = Message.Format.MESSAGE_FORMAT)
Expand All @@ -440,6 +430,16 @@ public interface ActiveMQClientLogger extends BasicLogger {
format = Message.Format.MESSAGE_FORMAT)
void connectionFactoryParameterIgnored(String parameterName);

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212079, value = "IoUring is not available, please add to the classpath or configure useIoUring=false to remove this warning",
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckIoUringAvailabilitynoClass();

@LogMessage(level = Logger.Level.WARN)
@Message(id = 212080, value = "Unable to check IoUring availability ",
format = Message.Format.MESSAGE_FORMAT)
void unableToCheckIoUringAvailability(@Cause Throwable e);

@LogMessage(level = Logger.Level.ERROR)
@Message(id = 214000, value = "Failed to call onMessage", format = Message.Format.MESSAGE_FORMAT)
void onMessageError(@Cause Throwable e);
Expand Down
6 changes: 3 additions & 3 deletions ...src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/CheckDependencies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import io.netty.channel.epoll.Epoll;
import io.netty.channel.kqueue.KQueue;
import io.netty.incubator.channel.uring.IOUring;
import org.apache.activemq.artemis.core.client.ActiveMQClientLogger;
import org.apache.activemq.artemis.utils.Env;
import org.jboss.logging.Logger;
Expand Down Expand Up @@ -55,10 +56,9 @@ public static final boolean isKQueueAvailable() {
}
}

public static boolean isIoUringAvailable() {
public static boolean isIOUringAvailable() {
try {
return Env.isLinuxOs() && (boolean) (Class.forName("io.netty.incubator.channel.uring.IOUring")
.getMethod("isAvailable").invoke(null));
return Env.isLinuxOs() && IOUring.isAvailable();
} catch (NoClassDefFoundError noClassDefFoundError) {
ActiveMQClientLogger.LOGGER.unableToCheckIoUringAvailabilitynoClass();
return false;
Expand Down
2 changes: 1 addition & 1 deletion ...rc/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class TransportConstants {

public static final String USE_KQUEUE_PROP_NAME = "useKQueue";

public static final String USE_IOURING_PROP_NAME = "useIoUring";
public static final String USE_IOURING_PROP_NAME = "useIOUring";

@Deprecated
/**
Expand Down
1 change: 1 addition & 0 deletions artemis-distribution/src/main/assembly/dep.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<include>org.apache.commons:commons-dbcp2</include>
<include>org.apache.commons:commons-pool2</include>
<include>io.netty:netty-all</include>
<include>io.netty.incubator:netty-incubator-transport-native-io_uring</include>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I similarly think its premature to distribute it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, leave it on a branch till there is a non incubator netty dependency.

<include>org.apache.qpid:proton-j</include>
<include>org.apache.activemq:activemq-client</include>
<include>org.apache.activemq:activemq-openwire-legacy</include>
Expand Down
1 change: 1 addition & 0 deletions artemis-features/src/main/resources/features.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<bundle>mvn:io.netty/netty-transport-native-epoll/${netty.version}</bundle>
<bundle>mvn:io.netty/netty-transport-native-kqueue/${netty.version}</bundle>
<bundle>mvn:io.netty/netty-transport-native-unix-common/${netty.version}</bundle>
<bundle>mvn:io.netty.incubator/netty-incubator-transport-native-io_uring/${netty.io_uring.version}</bundle>
<bundle>mvn:io.netty/netty-codec-http/${netty.version}</bundle>
</feature>

Expand Down
36 changes: 17 additions & 19 deletions ...ver/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
import io.netty.channel.socket.nio.NioServerSocketChannel;
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslHandler;
import io.netty.incubator.channel.uring.IOUringEventLoopGroup;
import io.netty.incubator.channel.uring.IOUringServerSocketChannel;
import io.netty.util.ResourceLeakDetector;
import io.netty.util.concurrent.GenericFutureListener;
import io.netty.util.concurrent.GlobalEventExecutor;
Expand Down Expand Up @@ -113,7 +115,7 @@ public class NettyAcceptor extends AbstractAcceptor {
public static final String NIO_ACCEPTOR_TYPE = "NIO";
public static final String EPOLL_ACCEPTOR_TYPE = "EPOLL";
public static final String KQUEUE_ACCEPTOR_TYPE = "KQUEUE";
public static final String IOURING_ACCEPTOR_TYPE = "IO_URING";
public static final String IOURING_ACCEPTOR_TYPE = "IOURING";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the existing name was fine, nicer even. Per the other PR, this doesnt seem to be used for URL config, with the other boolean toggle is, so I'm not sure it matters if it has the underscore, which is actually part of the io_uring name.


static {
// Disable default Netty leak detection if the Netty leak detection level system properties are not in use
Expand Down Expand Up @@ -150,7 +152,7 @@ public class NettyAcceptor extends AbstractAcceptor {

private final boolean useKQueue;

private final boolean useIoUring;
private final boolean useIOUring;

private final ProtocolHandler protocolHandler;

Expand Down Expand Up @@ -272,7 +274,7 @@ public NettyAcceptor(final String name,

useEpoll = ConfigurationHelper.getBooleanProperty(TransportConstants.USE_EPOLL_PROP_NAME, TransportConstants.DEFAULT_USE_EPOLL, configuration);
useKQueue = ConfigurationHelper.getBooleanProperty(TransportConstants.USE_KQUEUE_PROP_NAME, TransportConstants.DEFAULT_USE_KQUEUE, configuration);
useIoUring = ConfigurationHelper.getBooleanProperty(TransportConstants.USE_IOURING_PROP_NAME, TransportConstants.DEFAULT_USE_IOURING, configuration);
useIOUring = ConfigurationHelper.getBooleanProperty(TransportConstants.USE_IOURING_PROP_NAME, TransportConstants.DEFAULT_USE_IOURING, configuration);

backlog = ConfigurationHelper.getIntProperty(TransportConstants.BACKLOG_PROP_NAME, -1, configuration);
useInvm = ConfigurationHelper.getBooleanProperty(TransportConstants.USE_INVM_PROP_NAME, TransportConstants.DEFAULT_USE_INVM, configuration);
Expand Down Expand Up @@ -384,7 +386,18 @@ public synchronized void start() throws Exception {
remotingThreads = Runtime.getRuntime().availableProcessors() * 3;
}

if (useEpoll && CheckDependencies.isEpollAvailable()) {
if (useIOUring && CheckDependencies.isIOUringAvailable()) {
channelClazz = IOUringServerSocketChannel.class;
eventLoopGroup = new IOUringEventLoopGroup(remotingThreads, AccessController.doPrivileged(new PrivilegedAction<ActiveMQThreadFactory>() {
@Override
public ActiveMQThreadFactory run() {
return new ActiveMQThreadFactory("activemq-netty-threads", true, ClientSessionFactoryImpl.class.getClassLoader());
}
}));
acceptorType = IOURING_ACCEPTOR_TYPE;

logger.debug("Acceptor using native io_uring");
} else if (useEpoll && CheckDependencies.isEpollAvailable()) {
channelClazz = EpollServerSocketChannel.class;
eventLoopGroup = new EpollEventLoopGroup(remotingThreads, AccessController.doPrivileged(new PrivilegedAction<ActiveMQThreadFactory>() {
@Override
Expand All @@ -406,21 +419,6 @@ public ActiveMQThreadFactory run() {
acceptorType = KQUEUE_ACCEPTOR_TYPE;

logger.debug("Acceptor using native kqueue");
} else if (useIoUring && CheckDependencies.isIoUringAvailable()) {
channelClazz = (Class<? extends ServerChannel>) Class.forName("io.netty.incubator.channel.uring.IOUringServerSocketChannel",
true, ClientSessionFactoryImpl.class.getClassLoader());
eventLoopGroup = (EventLoopGroup) Class.forName("io.netty.incubator.channel.uring.IOUringEventLoopGroup",
true, ClientSessionFactoryImpl.class.getClassLoader())
.getConstructor(int.class, ThreadFactory.class)
.newInstance(remotingThreads, AccessController.doPrivileged(new PrivilegedAction<ActiveMQThreadFactory>() {
@Override
public ActiveMQThreadFactory run() {
return new ActiveMQThreadFactory("activemq-netty-threads", true, ClientSessionFactoryImpl.class.getClassLoader());
}
}));
acceptorType = IOURING_ACCEPTOR_TYPE;

logger.debug("Acceptor using native io_uring");
} else {
channelClazz = NioServerSocketChannel.class;
eventLoopGroup = new NioEventLoopGroup(remotingThreads, AccessController.doPrivileged(new PrivilegedAction<ActiveMQThreadFactory>() {
Expand Down
18 changes: 18 additions & 0 deletions docs/user-manual/en/configuring-transports.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,24 @@ The following properties are specific to this native transport:
a 64bit JVM is detected. Setting this to `false` will force the use of Java
NIO instead of epoll. Default is `true`

#### IO_Uring Native Transport
> **Note:**
>
> This is an **INCUBATOR** transport by netty and should be treated as such.
> This means it has not had thorough testing at this stage, and users that wish to use this should do their own full testing before using.
>
> We do encourage any feedback on this incubator transport, along with any performance reports positive or negative.
>
> @see https://github.com/netty/netty-incubator-transport-io_uring

On supported platforms IO_Uring can be used @see https://en.wikipedia.org/wiki/Io_uring
Copy link

@gemmellr gemmellr Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supported platforms could be less vague, i.e its Linux (EDIT: which is then mentioned below...it seems superfluous to have both).

I believe the name is simply io_uring rather than IO_Uring (applies here and most other places IO_Uring is used)


The following properties are specific to this native transport:

- `useIOUring` enables the use of io_uring if a supported linux platform is running
a 64bit JVM is detected and the netty io_uring dependency is available on the classpath.
Setting this to `true` will enable the use of IO_Uring. Default is `false`

#### MacOS Native Transport

On supported MacOS platforms KQueue is used, @see
Expand Down
9 changes: 9 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
<mockito.version>3.3.3</mockito.version>
<jctools.version>2.1.2</jctools.version>
<netty.version>4.1.59.Final</netty.version>
<netty.io_uring.version>0.0.8.Final</netty.io_uring.version>

<!-- this is basically for tests -->
<netty-tcnative-version>2.0.33.Final</netty-tcnative-version>
Expand Down Expand Up @@ -211,6 +212,7 @@

<netty-transport-native-epoll-classifier>linux-x86_64</netty-transport-native-epoll-classifier>
<netty-transport-native-kqueue-classifier>osx-x86_64</netty-transport-native-kqueue-classifier>
<netty-transport-native-io_uring-classifier>linux-x86_64</netty-transport-native-io_uring-classifier>

<!-- Ignore failed tests by default because there are "known" failures in the full test-suite.
This will be set to false for the "fast-tests" profile as none of those tests should fail. -->
Expand Down Expand Up @@ -625,6 +627,13 @@
<classifier>${netty-transport-native-kqueue-classifier}</classifier>
<!-- License: Apache 2.0 -->
</dependency>
<dependency>
<groupId>io.netty.incubator</groupId>
<artifactId>netty-incubator-transport-native-io_uring</artifactId>
<version>${netty.io_uring.version}</version>
<classifier>${netty-transport-native-io_uring-classifier}</classifier>
<!-- License: Apache 2.0 -->
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-codec-http</artifactId>
Expand Down