This is an automated email from the ASF dual-hosted git repository. sankarh pushed a commit to branch branch-3 in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/branch-3 by this push: new 005bac98c32 HIVE-27715: Backport of HIVE-25235: Remove ThreadPoolExecutorWithOomHook to branch-3 (David Mollitor reviewed by Miklos Gergely, Zhihua Deng) 005bac98c32 is described below commit 005bac98c32328eb2a87ba3123b6906abc861396 Author: Aman Raj <104416558+amanraj2...@users.noreply.github.com> AuthorDate: Fri Sep 22 09:32:32 2023 +0530 HIVE-27715: Backport of HIVE-25235: Remove ThreadPoolExecutorWithOomHook to branch-3 (David Mollitor reviewed by Miklos Gergely, Zhihua Deng) Signed-off-by: Sankar Hariappan <sank...@apache.org> Closes (#4729) --- .../hive/service/cli/session/SessionManager.java | 2 +- .../cli/thrift/EmbeddedThriftBinaryCLIService.java | 2 +- .../cli/thrift/ThreadPoolExecutorWithOomHook.java | 55 ---------------------- .../service/cli/thrift/ThriftBinaryCLIService.java | 11 ++--- .../service/cli/thrift/ThriftHttpCLIService.java | 10 ++-- .../apache/hive/service/server/HiveServer2.java | 11 ++--- .../hive/service/auth/TestPlainSaslHelper.java | 2 +- .../cli/session/TestPluggableHiveSessionImpl.java | 4 +- .../cli/session/TestSessionGlobalInitFile.java | 2 +- 9 files changed, 18 insertions(+), 81 deletions(-) diff --git a/service/src/java/org/apache/hive/service/cli/session/SessionManager.java b/service/src/java/org/apache/hive/service/cli/session/SessionManager.java index 277519cba5a..6244d76b4c2 100644 --- a/service/src/java/org/apache/hive/service/cli/session/SessionManager.java +++ b/service/src/java/org/apache/hive/service/cli/session/SessionManager.java @@ -202,7 +202,7 @@ public class SessionManager extends CompositeService { // Threads terminate when they are idle for more than the keepAliveTime // A bounded blocking queue is used to queue incoming operations, if #operations > poolSize String threadPoolName = "HiveServer2-Background-Pool"; - final BlockingQueue queue = new LinkedBlockingQueue<Runnable>(poolQueueSize); + final BlockingQueue<Runnable> queue = new LinkedBlockingQueue<Runnable>(poolQueueSize); backgroundOperationPool = new ThreadPoolExecutor(poolSize, poolSize, keepAliveTime, TimeUnit.SECONDS, queue, new ThreadFactoryWithGarbageCleanup(threadPoolName)); diff --git a/service/src/java/org/apache/hive/service/cli/thrift/EmbeddedThriftBinaryCLIService.java b/service/src/java/org/apache/hive/service/cli/thrift/EmbeddedThriftBinaryCLIService.java index 7ab7aee7b00..1d2b0e67911 100644 --- a/service/src/java/org/apache/hive/service/cli/thrift/EmbeddedThriftBinaryCLIService.java +++ b/service/src/java/org/apache/hive/service/cli/thrift/EmbeddedThriftBinaryCLIService.java @@ -33,7 +33,7 @@ public class EmbeddedThriftBinaryCLIService extends ThriftBinaryCLIService { public EmbeddedThriftBinaryCLIService() { // The non-test path that allows connections for the embedded service. - super(new CLIService(null, true), null); + super(new CLIService(null, true)); isEmbedded = true; HiveConf.setLoadHiveServer2Config(true); } diff --git a/service/src/java/org/apache/hive/service/cli/thrift/ThreadPoolExecutorWithOomHook.java b/service/src/java/org/apache/hive/service/cli/thrift/ThreadPoolExecutorWithOomHook.java deleted file mode 100644 index 1d2426235a7..00000000000 --- a/service/src/java/org/apache/hive/service/cli/thrift/ThreadPoolExecutorWithOomHook.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.hive.service.cli.thrift; - -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.Future; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; - -final class ThreadPoolExecutorWithOomHook extends ThreadPoolExecutor { - private final Runnable oomHook; - - public ThreadPoolExecutorWithOomHook(int corePoolSize, int maximumPoolSize, long keepAliveTime, - TimeUnit unit, BlockingQueue<Runnable> workQueue, ThreadFactory threadFactory, - Runnable oomHook) { - super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, threadFactory); - this.oomHook = oomHook; - } - - @Override - protected void afterExecute(Runnable r, Throwable t) { - super.afterExecute(r, t); - if (t == null && r instanceof Future<?>) { - try { - Future<?> future = (Future<?>) r; - if (future.isDone()) { - future.get(); - } - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - } catch (Throwable t2) { - t = t2; - } - } - if (t instanceof OutOfMemoryError) { - oomHook.run(); - } - } -} \ No newline at end of file diff --git a/service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java b/service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java index df2d3a7b719..31b604f4644 100644 --- a/service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java +++ b/service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import org.apache.hadoop.hive.common.auth.HiveAuthUtils; @@ -50,12 +51,10 @@ import org.apache.thrift.transport.TTransportFactory; public class ThriftBinaryCLIService extends ThriftCLIService { - private final Runnable oomHook; protected TServer server; - public ThriftBinaryCLIService(CLIService cliService, Runnable oomHook) { + public ThriftBinaryCLIService(CLIService cliService) { super(cliService, ThriftBinaryCLIService.class.getSimpleName()); - this.oomHook = oomHook; } @Override @@ -63,9 +62,9 @@ public class ThriftBinaryCLIService extends ThriftCLIService { try { // Server thread pool String threadPoolName = "HiveServer2-Handler-Pool"; - ExecutorService executorService = new ThreadPoolExecutorWithOomHook(minWorkerThreads, maxWorkerThreads, - workerKeepAliveTime, TimeUnit.SECONDS, new SynchronousQueue<Runnable>(), - new ThreadFactoryWithGarbageCleanup(threadPoolName), oomHook); + ExecutorService executorService = new ThreadPoolExecutor(minWorkerThreads, maxWorkerThreads, + workerKeepAliveTime, TimeUnit.SECONDS, new SynchronousQueue<>(), + new ThreadFactoryWithGarbageCleanup(threadPoolName)); // Thrift configs hiveAuthFactory = new HiveAuthFactory(hiveConf); diff --git a/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java b/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java index 89271d7020c..01417c72dfd 100644 --- a/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java +++ b/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java @@ -19,9 +19,9 @@ package org.apache.hive.service.cli.thrift; import java.util.Arrays; -import java.util.concurrent.ExecutorService; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.ThreadPoolExecutor; import javax.ws.rs.HttpMethod; @@ -59,10 +59,8 @@ public class ThriftHttpCLIService extends ThriftCLIService { private static final String APPLICATION_THRIFT = "application/x-thrift"; protected org.eclipse.jetty.server.Server server; - private final Runnable oomHook; - public ThriftHttpCLIService(CLIService cliService, Runnable oomHook) { + public ThriftHttpCLIService(CLIService cliService) { super(cliService, ThriftHttpCLIService.class.getSimpleName()); - this.oomHook = oomHook; } /** @@ -77,9 +75,9 @@ public class ThriftHttpCLIService extends ThriftCLIService { // Start with minWorkerThreads, expand till maxWorkerThreads and reject // subsequent requests String threadPoolName = "HiveServer2-HttpHandler-Pool"; - ExecutorService executorService = new ThreadPoolExecutorWithOomHook(minWorkerThreads, + ThreadPoolExecutor executorService = new ThreadPoolExecutor(minWorkerThreads, maxWorkerThreads,workerKeepAliveTime, TimeUnit.SECONDS, - new SynchronousQueue<Runnable>(), new ThreadFactoryWithGarbageCleanup(threadPoolName), oomHook); + new SynchronousQueue<Runnable>(), new ThreadFactoryWithGarbageCleanup(threadPoolName)); ExecutorThreadPool threadPool = new ExecutorThreadPool(executorService); // HTTP Server diff --git a/service/src/java/org/apache/hive/service/server/HiveServer2.java b/service/src/java/org/apache/hive/service/server/HiveServer2.java index 4aae495338f..2ba61371c52 100644 --- a/service/src/java/org/apache/hive/service/server/HiveServer2.java +++ b/service/src/java/org/apache/hive/service/server/HiveServer2.java @@ -210,16 +210,11 @@ public class HiveServer2 extends CompositeService { cliService = new CLIService(this, false); addService(cliService); final HiveServer2 hiveServer2 = this; - Runnable oomHook = new Runnable() { - @Override - public void run() { - hiveServer2.stop(); - } - }; + if (isHTTPTransportMode(hiveConf)) { - thriftCLIService = new ThriftHttpCLIService(cliService, oomHook); + thriftCLIService = new ThriftHttpCLIService(cliService); } else { - thriftCLIService = new ThriftBinaryCLIService(cliService, oomHook); + thriftCLIService = new ThriftBinaryCLIService(cliService); } addService(thriftCLIService); super.init(hiveConf); diff --git a/service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java b/service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java index 8bfa7dc157d..72548c37aa9 100644 --- a/service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java +++ b/service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java @@ -44,7 +44,7 @@ public class TestPlainSaslHelper extends TestCase { CLIService cliService = new CLIService(null, true); cliService.init(hconf); - ThriftCLIService tcliService = new ThriftBinaryCLIService(cliService, null); + ThriftCLIService tcliService = new ThriftBinaryCLIService(cliService); tcliService.init(hconf); TProcessorFactory procFactory = PlainSaslHelper.getPlainProcessorFactory(tcliService); assertEquals("doAs enabled processor for unsecure mode", diff --git a/service/src/test/org/apache/hive/service/cli/session/TestPluggableHiveSessionImpl.java b/service/src/test/org/apache/hive/service/cli/session/TestPluggableHiveSessionImpl.java index ef096202a12..cbc15ddbec6 100644 --- a/service/src/test/org/apache/hive/service/cli/session/TestPluggableHiveSessionImpl.java +++ b/service/src/test/org/apache/hive/service/cli/session/TestPluggableHiveSessionImpl.java @@ -44,7 +44,7 @@ public class TestPluggableHiveSessionImpl { CLIService cliService = new CLIService(null, true); cliService.init(hiveConf); - ThriftBinaryCLIService service = new ThriftBinaryCLIService(cliService, null); + ThriftBinaryCLIService service = new ThriftBinaryCLIService(cliService); service.init(hiveConf); ThriftCLIServiceClient client = new ThriftCLIServiceClient(service); @@ -70,7 +70,7 @@ public class TestPluggableHiveSessionImpl { CLIService cliService = new CLIService(null, true); cliService.init(hiveConf); - ThriftBinaryCLIService service = new ThriftBinaryCLIService(cliService, null); + ThriftBinaryCLIService service = new ThriftBinaryCLIService(cliService); service.init(hiveConf); ThriftCLIServiceClient client = new ThriftCLIServiceClient(service); diff --git a/service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java b/service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java index 9d00ec43531..904d256142c 100644 --- a/service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java +++ b/service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java @@ -52,7 +52,7 @@ public class TestSessionGlobalInitFile extends TestCase { */ private class FakeEmbeddedThriftBinaryCLIService extends ThriftBinaryCLIService { public FakeEmbeddedThriftBinaryCLIService(HiveConf hiveConf) { - super(new CLIService(null, true), null); + super(new CLIService(null, true)); isEmbedded = true; cliService.init(hiveConf); cliService.start();