Repository: netty/netty
Issue: Tail tasks queue: configure separately from tasks queue

IO transports (primarily epoll, but this also applies to kqueue, nio) cant be configured with separate `tail tasks` queue factory -
instead single queue factory is used for both normal tasks and tail tasks.

Their parent [SingleThreadEventLoop](https://github.com/netty/netty/blob/0d5774a82b470833c5f8a84fa4708142a4801f52/transport/src/main/java/io/netty/channel/SingleThreadEventLoop.java#L63) does have constructor accepting both `taskQueue` and `tailTaskQueue`.

I use `SingleThreadEventLoop.executeAfterEventLoopIteration(Runnable)` for internal tasks executed strictly from eventloop,
and would prefer to configure queue other than MPSC.

Issue comments:
@mostroverkhov SGTM, would you like to send a PR?

Pull request review comments:
nit: using `if (args.length > 3)` will remove redundancy for common arguments:

```java
if (args.length > 3) {
    taskQueueFactory = (EventLoopTaskQueueFactory) args[3];
}
if (args.length > 4) {
    tailTaskQueueFactory = (EventLoopTaskQueueFactory) args[4];
}
```
Javadoc?
How about adding a protected final getter method with the same name instead of exposing a field?
How about adding a protected final getter method with the same name instead of exposing a field?
There is always from 3 to 5 arguments for `EventLoopGroups` that exist in this project, so my thinking was that current form is simpler. Conditionals form looks better where there is wider range of optional arguments (>2) - can change if It is more convenient.


Most "non-convenience" constructors lacked javadocs, so I omitted them for uniformity - will add.
Child classes are defined in own packages outside `io.netty.util.concurrent`, so such getters are inaccessible by children test code - children would need helper method that only delegates to said getter.

see above comment
> Can we make the same changes to all `EventLoop(Group)` implementations such as `NioEventLoop(Group)`?

Yes, I will add them once approach for single implementation is accepted
Sounds good, just a nit so feel free to ignore.
Or for example
```
SingleThreadEventLoop {
       protected final Queue<Runnable> tailTaskQueue0() {}
}
```
```
EpollEventLoop extends SingleThreadEventLoop {
   /*package-private*/ final Queue<Runnable> tailTaskQueue() {
        return tailTaskQueue0();
    }
}
```
Is this what you are suggesting @trustin ?
Not thrilled about `protected` visibility for methods only required for tests, can we just have package local visibility?
Me neither - but java does not allow narrowing visibility in subclasses (keep in mind `SingleThreadEventExecutor` and `SingleThreadEventLoop` are in different packages). 

I know about 2 approaches for this - using single package across multiple modules so `package-private` members are accessible across whole project, or `internal` package with public visibility by default - with convention that only project code calls `internal` classes. However, `netty` follows neither ¯\_(ツ)_/¯ - logically related classes are scattered across multiple packages. Can you hint a simpler solution?

Given "mechanical" nature of this change (just exposing constructor parameter in child classes from parent) I am thinking about removing tests after verifying they pass - does It make sense? It seems every way of having this functionality tested is "awkward"  (under existing package organization)
> keep in mind SingleThreadEventExecutor and SingleThreadEventLoop are in different packages

Aah I didn't realize this, thanks.

> Given "mechanical" nature of this change (just exposing constructor parameter in child classes from parent) I am thinking about removing tests after verifying they pass - does It make sense?

Agreed, these tests aren't super useful specially when we have to expose `protected` methods for them.
Also put an `@UnstableApi` on it
+1... Let's just get rid of it then
nit: you can replace nested `if` blocks with sequential which makes it easier to read IMHO.

```java
if (argsLength > 3) {
             taskQueueFactory = (EventLoopTaskQueueFactory) args[3];
}
if (argsLength > 4) {
            tailTaskQueueFactory = (EventLoopTaskQueueFactory) args[4];
}
```

Patch that resolved the issue:
diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java
index c9cb838ed80..c3b2f6016ac 100644
--- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java
+++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java
@@ -86,8 +86,8 @@ public int get() throws Exception {
 
     EpollEventLoop(EventLoopGroup parent, Executor executor, int maxEvents,
                    SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler,
-                   EventLoopTaskQueueFactory queueFactory) {
-        super(parent, executor, false, newTaskQueue(queueFactory), newTaskQueue(queueFactory),
+                   EventLoopTaskQueueFactory taskQueueFactory, EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(parent, executor, false, newTaskQueue(taskQueueFactory), newTaskQueue(tailTaskQueueFactory),
                 rejectedExecutionHandler);
         selectStrategy = ObjectUtil.checkNotNull(strategy, "strategy");
         if (maxEvents == 0) {
diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
index 80de7fe0fad..ee1467d9149 100644
--- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
+++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
@@ -21,6 +21,7 @@
 import io.netty.channel.EventLoopTaskQueueFactory;
 import io.netty.channel.MultithreadEventLoopGroup;
 import io.netty.channel.SelectStrategyFactory;
+import io.netty.channel.SingleThreadEventLoop;
 import io.netty.util.concurrent.EventExecutorChooserFactory;
 import io.netty.util.concurrent.RejectedExecutionHandler;
 import io.netty.util.concurrent.RejectedExecutionHandlers;
@@ -134,6 +135,28 @@ public EpollEventLoopGroup(int nThreads, Executor executor, EventExecutorChooser
         super(nThreads, executor, chooserFactory, 0, selectStrategyFactory, rejectedExecutionHandler, queueFactory);
     }
 
+    /**
+     * @param nThreads the number of threads that will be used by this instance.
+     * @param executor the Executor to use, or {@code null} if default one should be used.
+     * @param chooserFactory the {@link EventExecutorChooserFactory} to use.
+     * @param selectStrategyFactory the {@link SelectStrategyFactory} to use.
+     * @param rejectedExecutionHandler the {@link RejectedExecutionHandler} to use.
+     * @param taskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                         {@link SingleThreadEventLoop#execute(Runnable)},
+     *                         or {@code null} if default one should be used.
+     * @param tailTaskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                             {@link SingleThreadEventLoop#executeAfterEventLoopIteration(Runnable)},
+     *                             or {@code null} if default one should be used.
+     */
+    public EpollEventLoopGroup(int nThreads, Executor executor, EventExecutorChooserFactory chooserFactory,
+                               SelectStrategyFactory selectStrategyFactory,
+                               RejectedExecutionHandler rejectedExecutionHandler,
+                               EventLoopTaskQueueFactory taskQueueFactory,
+                               EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(nThreads, executor, chooserFactory, 0, selectStrategyFactory, rejectedExecutionHandler, taskQueueFactory,
+                tailTaskQueueFactory);
+    }
+
     /**
      * @deprecated This method will be removed in future releases, and is not guaranteed to have any impacts.
      */
@@ -146,9 +169,21 @@ public void setIoRatio(int ioRatio) {
 
     @Override
     protected EventLoop newChild(Executor executor, Object... args) throws Exception {
-        EventLoopTaskQueueFactory queueFactory = args.length == 4 ? (EventLoopTaskQueueFactory) args[3] : null;
-        return new EpollEventLoop(this, executor, (Integer) args[0],
-                ((SelectStrategyFactory) args[1]).newSelectStrategy(),
-                (RejectedExecutionHandler) args[2], queueFactory);
+        Integer maxEvents = (Integer) args[0];
+        SelectStrategyFactory selectStrategyFactory = (SelectStrategyFactory) args[1];
+        RejectedExecutionHandler rejectedExecutionHandler = (RejectedExecutionHandler) args[2];
+        EventLoopTaskQueueFactory taskQueueFactory = null;
+        EventLoopTaskQueueFactory tailTaskQueueFactory = null;
+
+        int argsLength = args.length;
+        if (argsLength > 3) {
+            taskQueueFactory = (EventLoopTaskQueueFactory) args[3];
+        }
+        if (argsLength > 4) {
+            tailTaskQueueFactory = (EventLoopTaskQueueFactory) args[4];
+        }
+        return new EpollEventLoop(this, executor, maxEvents,
+                selectStrategyFactory.newSelectStrategy(),
+                rejectedExecutionHandler, taskQueueFactory, tailTaskQueueFactory);
     }
 }
diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java
index 6791768a7d2..cdd57b30ae8 100644
--- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java
+++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java
@@ -73,8 +73,8 @@ public int get() throws Exception {
 
     KQueueEventLoop(EventLoopGroup parent, Executor executor, int maxEvents,
                     SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler,
-                    EventLoopTaskQueueFactory queueFactory) {
-        super(parent, executor, false, newTaskQueue(queueFactory), newTaskQueue(queueFactory),
+                    EventLoopTaskQueueFactory taskQueueFactory, EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(parent, executor, false, newTaskQueue(taskQueueFactory), newTaskQueue(tailTaskQueueFactory),
                 rejectedExecutionHandler);
         this.selectStrategy = ObjectUtil.checkNotNull(strategy, "strategy");
         this.kqueueFd = Native.newKQueue();
diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoopGroup.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoopGroup.java
index 1be9de7574d..30c726da503 100644
--- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoopGroup.java
+++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoopGroup.java
@@ -20,6 +20,7 @@
 import io.netty.channel.EventLoopTaskQueueFactory;
 import io.netty.channel.MultithreadEventLoopGroup;
 import io.netty.channel.SelectStrategyFactory;
+import io.netty.channel.SingleThreadEventLoop;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.EventExecutorChooserFactory;
 import io.netty.util.concurrent.RejectedExecutionHandler;
@@ -133,6 +134,28 @@ public KQueueEventLoopGroup(int nThreads, Executor executor, EventExecutorChoose
                 rejectedExecutionHandler, queueFactory);
     }
 
+    /**
+     * @param nThreads the number of threads that will be used by this instance.
+     * @param executor the Executor to use, or {@code null} if default one should be used.
+     * @param chooserFactory the {@link EventExecutorChooserFactory} to use.
+     * @param selectStrategyFactory the {@link SelectStrategyFactory} to use.
+     * @param rejectedExecutionHandler the {@link RejectedExecutionHandler} to use.
+     * @param taskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                         {@link SingleThreadEventLoop#execute(Runnable)},
+     *                         or {@code null} if default one should be used.
+     * @param tailTaskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                             {@link SingleThreadEventLoop#executeAfterEventLoopIteration(Runnable)},
+     *                             or {@code null} if default one should be used.
+     */
+    public KQueueEventLoopGroup(int nThreads, Executor executor, EventExecutorChooserFactory chooserFactory,
+                               SelectStrategyFactory selectStrategyFactory,
+                               RejectedExecutionHandler rejectedExecutionHandler,
+                               EventLoopTaskQueueFactory taskQueueFactory,
+                               EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(nThreads, executor, chooserFactory, 0, selectStrategyFactory, rejectedExecutionHandler, taskQueueFactory,
+                tailTaskQueueFactory);
+    }
+
     /**
      * Sets the percentage of the desired amount of time spent for I/O in the child event loops.  The default value is
      * {@code 50}, which means the event loop will try to spend the same amount of time for I/O as for non-I/O tasks.
@@ -145,10 +168,21 @@ public void setIoRatio(int ioRatio) {
 
     @Override
     protected EventLoop newChild(Executor executor, Object... args) throws Exception {
-        EventLoopTaskQueueFactory queueFactory = args.length == 4 ? (EventLoopTaskQueueFactory) args[3] : null;
-
-        return new KQueueEventLoop(this, executor, (Integer) args[0],
-                ((SelectStrategyFactory) args[1]).newSelectStrategy(),
-                (RejectedExecutionHandler) args[2], queueFactory);
+        Integer maxEvents = (Integer) args[0];
+        SelectStrategyFactory selectStrategyFactory = (SelectStrategyFactory) args[1];
+        RejectedExecutionHandler rejectedExecutionHandler = (RejectedExecutionHandler) args[2];
+        EventLoopTaskQueueFactory taskQueueFactory = null;
+        EventLoopTaskQueueFactory tailTaskQueueFactory = null;
+
+        int argsLength = args.length;
+        if (argsLength > 3) {
+            taskQueueFactory = (EventLoopTaskQueueFactory) args[3];
+        }
+        if (argsLength > 4) {
+            tailTaskQueueFactory = (EventLoopTaskQueueFactory) args[4];
+        }
+        return new KQueueEventLoop(this, executor, maxEvents,
+                selectStrategyFactory.newSelectStrategy(),
+                rejectedExecutionHandler, taskQueueFactory, tailTaskQueueFactory);
     }
 }
diff --git a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
index 5acdff64d5f..719037d9417 100644
--- a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
+++ b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
@@ -134,8 +134,8 @@ public Void run() {
 
     NioEventLoop(NioEventLoopGroup parent, Executor executor, SelectorProvider selectorProvider,
                  SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler,
-                 EventLoopTaskQueueFactory queueFactory) {
-        super(parent, executor, false, newTaskQueue(queueFactory), newTaskQueue(queueFactory),
+                 EventLoopTaskQueueFactory taskQueueFactory, EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(parent, executor, false, newTaskQueue(taskQueueFactory), newTaskQueue(tailTaskQueueFactory),
                 rejectedExecutionHandler);
         this.provider = ObjectUtil.checkNotNull(selectorProvider, "selectorProvider");
         this.selectStrategy = ObjectUtil.checkNotNull(strategy, "selectStrategy");
diff --git a/transport/src/main/java/io/netty/channel/nio/NioEventLoopGroup.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoopGroup.java
index 89f7b40bf1b..9854e9c03c0 100644
--- a/transport/src/main/java/io/netty/channel/nio/NioEventLoopGroup.java
+++ b/transport/src/main/java/io/netty/channel/nio/NioEventLoopGroup.java
@@ -16,11 +16,12 @@
 package io.netty.channel.nio;
 
 import io.netty.channel.Channel;
-import io.netty.channel.EventLoop;
 import io.netty.channel.DefaultSelectStrategyFactory;
+import io.netty.channel.EventLoop;
 import io.netty.channel.EventLoopTaskQueueFactory;
 import io.netty.channel.MultithreadEventLoopGroup;
 import io.netty.channel.SelectStrategyFactory;
+import io.netty.channel.SingleThreadEventLoop;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.EventExecutorChooserFactory;
 import io.netty.util.concurrent.RejectedExecutionHandler;
@@ -119,6 +120,30 @@ public NioEventLoopGroup(int nThreads, Executor executor, EventExecutorChooserFa
                 rejectedExecutionHandler, taskQueueFactory);
     }
 
+    /**
+     * @param nThreads the number of threads that will be used by this instance.
+     * @param executor the Executor to use, or {@code null} if default one should be used.
+     * @param chooserFactory the {@link EventExecutorChooserFactory} to use.
+     * @param selectorProvider the {@link SelectorProvider} to use.
+     * @param selectStrategyFactory the {@link SelectStrategyFactory} to use.
+     * @param rejectedExecutionHandler the {@link RejectedExecutionHandler} to use.
+     * @param taskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                         {@link SingleThreadEventLoop#execute(Runnable)},
+     *                         or {@code null} if default one should be used.
+     * @param tailTaskQueueFactory the {@link EventLoopTaskQueueFactory} to use for
+     *                             {@link SingleThreadEventLoop#executeAfterEventLoopIteration(Runnable)},
+     *                             or {@code null} if default one should be used.
+     */
+    public NioEventLoopGroup(int nThreads, Executor executor, EventExecutorChooserFactory chooserFactory,
+                             SelectorProvider selectorProvider,
+                             SelectStrategyFactory selectStrategyFactory,
+                             RejectedExecutionHandler rejectedExecutionHandler,
+                             EventLoopTaskQueueFactory taskQueueFactory,
+                             EventLoopTaskQueueFactory tailTaskQueueFactory) {
+        super(nThreads, executor, chooserFactory, selectorProvider, selectStrategyFactory,
+                rejectedExecutionHandler, taskQueueFactory, tailTaskQueueFactory);
+    }
+
     /**
      * Sets the percentage of the desired amount of time spent for I/O in the child event loops.  The default value is
      * {@code 50}, which means the event loop will try to spend the same amount of time for I/O as for non-I/O tasks.
@@ -141,8 +166,21 @@ public void rebuildSelectors() {
 
     @Override
     protected EventLoop newChild(Executor executor, Object... args) throws Exception {
-        EventLoopTaskQueueFactory queueFactory = args.length == 4 ? (EventLoopTaskQueueFactory) args[3] : null;
-        return new NioEventLoop(this, executor, (SelectorProvider) args[0],
-            ((SelectStrategyFactory) args[1]).newSelectStrategy(), (RejectedExecutionHandler) args[2], queueFactory);
+        SelectorProvider selectorProvider = (SelectorProvider) args[0];
+        SelectStrategyFactory selectStrategyFactory = (SelectStrategyFactory) args[1];
+        RejectedExecutionHandler rejectedExecutionHandler = (RejectedExecutionHandler) args[2];
+        EventLoopTaskQueueFactory taskQueueFactory = null;
+        EventLoopTaskQueueFactory tailTaskQueueFactory = null;
+
+        int argsLength = args.length;
+        if (argsLength > 3) {
+            taskQueueFactory = (EventLoopTaskQueueFactory) args[3];
+        }
+        if (argsLength > 4) {
+            tailTaskQueueFactory = (EventLoopTaskQueueFactory) args[4];
+        }
+        return new NioEventLoop(this, executor, selectorProvider,
+                selectStrategyFactory.newSelectStrategy(),
+                rejectedExecutionHandler, taskQueueFactory, tailTaskQueueFactory);
     }
 }


Test patch:
diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java
index 02e55fd6aaa..f3d42ae3f24 100644
--- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java
+++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java
@@ -15,13 +15,13 @@
  */
 package io.netty.channel.epoll;
 
+import io.netty.testsuite.transport.AbstractSingleThreadEventLoopTest;
 import io.netty.channel.DefaultSelectStrategyFactory;
 import io.netty.channel.EventLoop;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.ServerChannel;
 import io.netty.channel.socket.ServerSocketChannel;
 import io.netty.channel.unix.FileDescriptor;
-import io.netty.testsuite.transport.AbstractSingleThreadEventLoopTest;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.RejectedExecutionHandlers;
@@ -61,7 +61,8 @@ public void testScheduleBigDelayNotOverflow() {
 
         final EventLoopGroup group = new EpollEventLoop(null,
                 new ThreadPerTaskExecutor(new DefaultThreadFactory(getClass())), 0,
-                DefaultSelectStrategyFactory.INSTANCE.newSelectStrategy(), RejectedExecutionHandlers.reject(), null) {
+                DefaultSelectStrategyFactory.INSTANCE.newSelectStrategy(), RejectedExecutionHandlers.reject(),
+                null, null) {
             @Override
             void handleLoopException(Throwable t) {
                 capture.set(t);