Merge pull request cdapio#99 from cdapio/feature/same-thread-event

Update the EventExecutor behavior

Author: chtyim

src/main/java/io/cdap/http/NettyHttpService.java

@@ -17,6 +17,8 @@
 package io.cdap.http;
 
 import io.cdap.http.internal.BasicHandlerContext;
+import io.cdap.http.internal.ForwardingEventExecutorGroup;
+import io.cdap.http.internal.ForwardingOrderedEventExecutor;
 import io.cdap.http.internal.HttpDispatcher;
 import io.cdap.http.internal.HttpResourceHandler;
 import io.cdap.http.internal.NonStickyEventExecutorGroup;
@@ -37,9 +39,11 @@
 import io.netty.handler.codec.http.HttpServerKeepAliveHandler;
 import io.netty.handler.ssl.SslHandler;
 import io.netty.handler.stream.ChunkedWriteHandler;
+import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.EventExecutorGroup;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.ImmediateEventExecutor;
+import io.netty.util.concurrent.OrderedEventExecutor;
 import io.netty.util.concurrent.UnorderedThreadPoolEventExecutor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,6 +53,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.RejectedExecutionHandler;
@@ -283,8 +288,9 @@ private EventExecutorGroup createEventExecutorGroup(int size) {
       return null;
     }
 
+    ThreadGroup threadGroup = new ThreadGroup(serviceName + "-executor-thread");
+
     ThreadFactory threadFactory = new ThreadFactory() {
-      private final ThreadGroup threadGroup = new ThreadGroup(serviceName + "-executor-thread");
       private final AtomicLong count = new AtomicLong(0);
 
       @Override
@@ -301,7 +307,68 @@ public Thread newThread(Runnable r) {
       executor.setKeepAliveTime(execThreadKeepAliveSecs, TimeUnit.SECONDS);
       executor.allowCoreThreadTimeOut(true);
     }
-    return new NonStickyEventExecutorGroup(executor);
+
+    // Returns a EventExecutorGroup that overrides the inEventLoop behavior of the EventExecutor returned by the group.
+    // The inEventLoop will return true if it is calling from a thread in the same thread group, hence was created
+    // from the same executor.
+    // This is to make sure channel events coming from a thread owned by the EventExecutor
+    // will be executed in the same thread instead of submitting a new task.
+    return new ForwardingEventExecutorGroup(new NonStickyEventExecutorGroup(executor)) {
+
+      private final EventExecutorGroup group = this;
+
+      @Override
+      public EventExecutor next() {
+        return wrapEventExecutor(super.next());
+      }
+
+      @Override
+      public Iterator<EventExecutor> iterator() {
+        Iterator<EventExecutor> iterator = super.iterator();
+
+        return new Iterator<EventExecutor>() {
+          @Override
+          public boolean hasNext() {
+            return iterator.hasNext();
+          }
+
+          @Override
+          public EventExecutor next() {
+            return wrapEventExecutor(iterator.next());
+          }
+
+          @Override
+          public void remove() {
+            iterator.remove();
+          }
+        };
+      }
+
+      private EventExecutor wrapEventExecutor(EventExecutor executor) {
+        if (!(executor instanceof OrderedEventExecutor)) {
+          // This should never happen since we use the NonStickyEventExecutorGroup above.
+          throw new IllegalStateException("The executor is not an OrderedEventExecutor: " + executor.getClass());
+        }
+
+        return new ForwardingOrderedEventExecutor((OrderedEventExecutor) executor) {
+
+          @Override
+          public EventExecutorGroup parent() {
+            return group;
+          }
+
+          @Override
+          public boolean inEventLoop() {
+            return inEventLoop(Thread.currentThread());
+          }
+
+          @Override
+          public boolean inEventLoop(Thread thread) {
+            return threadGroup == thread.getThreadGroup();
+          }
+        };
+      }
+    };
   }
 
   /**

src/main/java/io/cdap/http/internal/ForwardingEventExecutorGroup.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright © 2021 Cask Data, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package io.cdap.http.internal;
+
+import io.netty.util.concurrent.EventExecutor;
+import io.netty.util.concurrent.EventExecutorGroup;
+import io.netty.util.concurrent.Future;
+import io.netty.util.concurrent.ScheduledFuture;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+/**
+ * A {@link EventExecutorGroup} that forwards all methods to another {@link EventExecutorGroup}.
+ */
+public class ForwardingEventExecutorGroup implements EventExecutorGroup {
+
+  private final EventExecutorGroup delegate;
+
+  public ForwardingEventExecutorGroup(EventExecutorGroup delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  public boolean isShuttingDown() {
+    return delegate.isShuttingDown();
+  }
+
+  @Override
+  public Future<?> shutdownGracefully() {
+    return delegate.shutdownGracefully();
+  }
+
+  @Override
+  public Future<?> shutdownGracefully(long l, long l1, TimeUnit timeUnit) {
+    return delegate.shutdownGracefully(l, l1, timeUnit);
+  }
+
+  @Override
+  public Future<?> terminationFuture() {
+    return delegate.terminationFuture();
+  }
+
+  @Override
+  @Deprecated
+  public void shutdown() {
+    delegate.shutdown();
+  }
+
+  @Override
+  @Deprecated
+  public List<Runnable> shutdownNow() {
+    return delegate.shutdownNow();
+  }
+
+  @Override
+  public EventExecutor next() {
+    return delegate.next();
+  }
+
+  @Override
+  public Iterator<EventExecutor> iterator() {
+    return delegate.iterator();
+  }
+
+  @Override
+  public Future<?> submit(Runnable runnable) {
+    return delegate.submit(runnable);
+  }
+
+  @Override
+  public <T> Future<T> submit(Runnable runnable, T t) {
+    return delegate.submit(runnable, t);
+  }
+
+  @Override
+  public <T> Future<T> submit(Callable<T> callable) {
+    return delegate.submit(callable);
+  }
+
+  @Override
+  public ScheduledFuture<?> schedule(Runnable runnable, long l, TimeUnit timeUnit) {
+    return delegate.schedule(runnable, l, timeUnit);
+  }
+
+  @Override
+  public <V> ScheduledFuture<V> schedule(Callable<V> callable, long l, TimeUnit timeUnit) {
+    return delegate.schedule(callable, l, timeUnit);
+  }
+
+  @Override
+  public ScheduledFuture<?> scheduleAtFixedRate(Runnable runnable, long l, long l1, TimeUnit timeUnit) {
+    return delegate.scheduleAtFixedRate(runnable, l, l1, timeUnit);
+  }
+
+  @Override
+  public ScheduledFuture<?> scheduleWithFixedDelay(Runnable runnable, long l, long l1, TimeUnit timeUnit) {
+    return delegate.scheduleWithFixedDelay(runnable, l, l1, timeUnit);
+  }
+
+  @Override
+  public boolean isShutdown() {
+    return delegate.isShutdown();
+  }
+
+  @Override
+  public boolean isTerminated() {
+    return delegate.isTerminated();
+  }
+
+  @Override
+  public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException {
+    return delegate.awaitTermination(timeout, unit);
+  }
+
+  @Override
+  public <T> List<java.util.concurrent.Future<T>> invokeAll(Collection<? extends Callable<T>> tasks)
+    throws InterruptedException {
+    return delegate.invokeAll(tasks);
+  }
+
+  @Override
+  public <T> List<java.util.concurrent.Future<T>> invokeAll(Collection<? extends Callable<T>> tasks,
+                                                            long timeout, TimeUnit unit) throws InterruptedException {
+    return delegate.invokeAll(tasks, timeout, unit);
+  }
+
+  @Override
+  public <T> T invokeAny(Collection<? extends Callable<T>> tasks) throws InterruptedException, ExecutionException {
+    return delegate.invokeAny(tasks);
+  }
+
+  @Override
+  public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit)
+    throws InterruptedException, ExecutionException, TimeoutException {
+    return delegate.invokeAny(tasks, timeout, unit);
+  }
+
+  @Override
+  public void execute(Runnable command) {
+    delegate.execute(command);
+  }
+}