You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by gu...@apache.org on 2021/02/21 08:38:23 UTC

[spark] branch branch-3.1 updated: [SPARK-34373][SQL] HiveThriftServer2 startWithContext may hang with a race issue

This is an automated email from the ASF dual-hosted git repository.

gurwls223 pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new 7ef5c1a  [SPARK-34373][SQL] HiveThriftServer2 startWithContext may hang with a race issue
7ef5c1a is described below

commit 7ef5c1a592f16c73e585b2df4879bd8fc8cea44e
Author: Kent Yao <ya...@apache.org>
AuthorDate: Sun Feb 21 17:37:12 2021 +0900

    [SPARK-34373][SQL] HiveThriftServer2 startWithContext may hang with a race issue
    
    ### What changes were proposed in this pull request?
    
    fix a race issue by interrupting the thread
    
    ### Why are the changes needed?
    
    ```
    21:43:26.809 WARN org.apache.thrift.server.TThreadPoolServer: Transport error occurred during acceptance of message.
    org.apache.thrift.transport.TTransportException: No underlying server socket.
    at org.apache.thrift.transport.TServerSocket.acceptImpl(TServerSocket.java:126)
    at org.apache.thrift.transport.TServerSocket.acceptImpl(TServerSocket.java:35)
    at org.apache.thrift.transport.TServerTransport.acceException in thread "Thread-15" java.io.IOException: Stream closed
    at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:170)
    at java.io.BufferedInputStream.read(BufferedInputStream.java:336)
    at java.io.FilterInputStream.read(FilterInputStream.java:107)
    at scala.sys.process.BasicIO$.loop$1(BasicIO.scala:238)
    at scala.sys.process.BasicIO$.transferFullyImpl(BasicIO.scala:246)
    at scala.sys.process.BasicIO$.transferFully(BasicIO.scala:227)
    at scala.sys.process.BasicIO$.$anonfun$toStdOut$1(BasicIO.scala:221)
    ```
    when the TServer try to `serve` after `stop`, it hangs with the log above forever
    ### Does this PR introduce _any_ user-facing change?
    
    no
    ### How was this patch tested?
    
    passing ci
    
    Closes #31479 from yaooqinn/SPARK-34373.
    
    Authored-by: Kent Yao <ya...@apache.org>
    Signed-off-by: HyukjinKwon <gu...@apache.org>
    (cherry picked from commit 1fac706db560001411672c5ade42f6608f82989e)
    Signed-off-by: HyukjinKwon <gu...@apache.org>
---
 .../service/cli/thrift/ThriftBinaryCLIService.java | 10 ++++++++
 .../hive/service/cli/thrift/ThriftCLIService.java  | 25 ++++++++------------
 .../service/cli/thrift/ThriftHttpCLIService.java   | 27 ++++++++++++++++++----
 3 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java
index ffca107..8085c8d 100644
--- a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java
+++ b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java
@@ -39,6 +39,7 @@ import org.apache.hive.service.server.ThreadFactoryWithGarbageCleanup;
 import org.apache.thrift.TException;
 import org.apache.thrift.TProcessorFactory;
 import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.server.TServer;
 import org.apache.thrift.server.TThreadPoolServer;
 import org.apache.thrift.transport.TServerSocket;
 import org.apache.thrift.transport.TTransportFactory;
@@ -46,6 +47,8 @@ import org.apache.thrift.transport.TTransportFactory;
 
 public class ThriftBinaryCLIService extends ThriftCLIService {
 
+  protected TServer server;
+
   public ThriftBinaryCLIService(CLIService cliService) {
     super(cliService, ThriftBinaryCLIService.class.getSimpleName());
   }
@@ -112,6 +115,13 @@ public class ThriftBinaryCLIService extends ThriftCLIService {
   }
 
   @Override
+  protected void stopServer() {
+    server.stop();
+    server = null;
+    LOG.info("Thrift server has stopped");
+  }
+
+  @Override
   public TGetQueryIdResp GetQueryId(TGetQueryIdReq req) throws TException {
     try {
       return new TGetQueryIdResp(cliService.getQueryId(req.getOperationHandle()));
diff --git a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
index 150f1d6..a65951c 100644
--- a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
+++ b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
@@ -40,7 +40,6 @@ import org.apache.hive.service.server.HiveServer2;
 import org.apache.thrift.TException;
 import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.server.ServerContext;
-import org.apache.thrift.server.TServer;
 import org.apache.thrift.server.TServerEventHandler;
 import org.apache.thrift.transport.TTransport;
 import org.slf4j.Logger;
@@ -61,8 +60,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe
   protected int portNum;
   protected InetAddress serverIPAddress;
   protected String hiveHost;
-  protected TServer server;
-  protected org.eclipse.jetty.server.Server httpServer;
+  private Thread serverThread = null;
 
   private boolean isStarted = false;
   protected boolean isEmbedded = false;
@@ -177,26 +175,23 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe
     super.start();
     if (!isStarted && !isEmbedded) {
       initializeServer();
-      new Thread(this).start();
+      serverThread = new Thread(this);
+      serverThread.setName(getName());
+      serverThread.start();
       isStarted = true;
     }
   }
 
+  protected abstract void stopServer();
+
   @Override
   public synchronized void stop() {
     if (isStarted && !isEmbedded) {
-      if(server != null) {
-        server.stop();
-        LOG.info("Thrift server has stopped");
-      }
-      if((httpServer != null) && httpServer.isStarted()) {
-        try {
-          httpServer.stop();
-          LOG.info("Http server has stopped");
-        } catch (Exception e) {
-          LOG.error("Error stopping Http server: ", e);
-        }
+      if (serverThread != null) {
+        serverThread.interrupt();
+        serverThread = null;
       }
+      stopServer();
       isStarted = false;
     }
     super.stop();
diff --git a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
index ab9ed5b..24e6d98 100644
--- a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
+++ b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
@@ -51,6 +51,8 @@ import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
 
 public class ThriftHttpCLIService extends ThriftCLIService {
 
+  protected org.eclipse.jetty.server.Server httpServer;
+
   public ThriftHttpCLIService(CLIService cliService) {
     super(cliService, ThriftHttpCLIService.class.getSimpleName());
   }
@@ -152,6 +154,19 @@ public class ThriftHttpCLIService extends ThriftCLIService {
     }
   }
 
+  @Override
+  protected void stopServer() {
+    if ((httpServer != null) && httpServer.isStarted()) {
+      try {
+        httpServer.stop();
+        httpServer = null;
+        LOG.info("Thrift HTTP server has been stopped");
+      } catch (Exception e) {
+        LOG.error("Error stopping HTTP server: ", e);
+      }
+    }
+  }
+
   /**
    * Configure Jetty to serve http requests. Example of a client connection URL:
    * http://localhost:10000/servlets/thrifths2/ A gateway may cause actual target URL to differ,
@@ -162,10 +177,14 @@ public class ThriftHttpCLIService extends ThriftCLIService {
     try {
       httpServer.join();
     } catch (Throwable t) {
-      LOG.error(
-          "Error starting HiveServer2: could not start "
-              + ThriftHttpCLIService.class.getSimpleName(), t);
-      System.exit(-1);
+      if (t instanceof InterruptedException) {
+        // This is likely a shutdown
+        LOG.info("Caught " + t.getClass().getSimpleName() + ". Shutting down thrift server.");
+      } else {
+        LOG.error("Error starting HiveServer2: could not start "
+            + ThriftHttpCLIService.class.getSimpleName(), t);
+        System.exit(-1);
+      }
     }
   }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org