Skip to content

Commit c11078b

Browse files
yaooqinndongjoon-hyun
authored andcommitted
[SPARK-32034][SQL] Port HIVE-14817: Shutdown the SessionManager timeoutChecker thread properly upon shutdown
### What changes were proposed in this pull request? This PR port https://issues.apache.org/jira/browse/HIVE-14817 for spark thrift server. ### Why are the changes needed? When stopping the HiveServer2, the non-daemon thread stops the server from terminating ```sql "HiveServer2-Background-Pool: Thread-79" #79 prio=5 os_prio=31 tid=0x00007fde26138800 nid=0x13713 waiting on condition [0x0000700010c32000] java.lang.Thread.State: TIMED_WAITING (sleeping) at java.lang.Thread.sleep(Native Method) at org.apache.hive.service.cli.session.SessionManager$1.sleepInterval(SessionManager.java:178) at org.apache.hive.service.cli.session.SessionManager$1.run(SessionManager.java:156) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748) ``` Here is an example to reproduce: https://github.com/yaooqinn/kyuubi/blob/master/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/spark/SparkSQLEngineApp.scala Also, it causes issues as HIVE-14817 described which ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Passing Jenkins Closes #28870 from yaooqinn/SPARK-32034. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 9f8e15b) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent 2f3618c commit c11078b

File tree

2 files changed

+48
-16
lines changed

2 files changed

+48
-16
lines changed

sql/hive-thriftserver/v1.2/src/main/java/org/apache/hive/service/cli/session/SessionManager.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,20 @@ public synchronized void start() {
148148
}
149149
}
150150

151+
private final Object timeoutCheckerLock = new Object();
152+
151153
private void startTimeoutChecker() {
152154
final long interval = Math.max(checkInterval, 3000L); // minimum 3 seconds
153-
Runnable timeoutChecker = new Runnable() {
155+
final Runnable timeoutChecker = new Runnable() {
154156
@Override
155157
public void run() {
156-
for (sleepInterval(interval); !shutdown; sleepInterval(interval)) {
158+
sleepFor(interval);
159+
while (!shutdown) {
157160
long current = System.currentTimeMillis();
158161
for (HiveSession session : new ArrayList<HiveSession>(handleToSession.values())) {
162+
if (shutdown) {
163+
break;
164+
}
159165
if (sessionTimeout > 0 && session.getLastAccessTime() + sessionTimeout <= current
160166
&& (!checkOperation || session.getNoOperationTime() > sessionTimeout)) {
161167
SessionHandle handle = session.getSessionHandle();
@@ -170,24 +176,34 @@ public void run() {
170176
session.closeExpiredOperations();
171177
}
172178
}
179+
sleepFor(interval);
173180
}
174181
}
175182

176-
private void sleepInterval(long interval) {
177-
try {
178-
Thread.sleep(interval);
179-
} catch (InterruptedException e) {
180-
// ignore
183+
private void sleepFor(long interval) {
184+
synchronized (timeoutCheckerLock) {
185+
try {
186+
timeoutCheckerLock.wait(interval);
187+
} catch (InterruptedException e) {
188+
// Ignore, and break.
189+
}
181190
}
182191
}
183192
};
184193
backgroundOperationPool.execute(timeoutChecker);
185194
}
186195

196+
private void shutdownTimeoutChecker() {
197+
shutdown = true;
198+
synchronized (timeoutCheckerLock) {
199+
timeoutCheckerLock.notify();
200+
}
201+
}
202+
187203
@Override
188204
public synchronized void stop() {
189205
super.stop();
190-
shutdown = true;
206+
shutdownTimeoutChecker();
191207
if (backgroundOperationPool != null) {
192208
backgroundOperationPool.shutdown();
193209
long timeout = hiveConf.getTimeVar(

sql/hive-thriftserver/v2.3/src/main/java/org/apache/hive/service/cli/session/SessionManager.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,20 @@ public synchronized void start() {
148148
}
149149
}
150150

151+
private final Object timeoutCheckerLock = new Object();
152+
151153
private void startTimeoutChecker() {
152154
final long interval = Math.max(checkInterval, 3000L); // minimum 3 seconds
153-
Runnable timeoutChecker = new Runnable() {
155+
final Runnable timeoutChecker = new Runnable() {
154156
@Override
155157
public void run() {
156-
for (sleepInterval(interval); !shutdown; sleepInterval(interval)) {
158+
sleepFor(interval);
159+
while (!shutdown) {
157160
long current = System.currentTimeMillis();
158161
for (HiveSession session : new ArrayList<HiveSession>(handleToSession.values())) {
162+
if (shutdown) {
163+
break;
164+
}
159165
if (sessionTimeout > 0 && session.getLastAccessTime() + sessionTimeout <= current
160166
&& (!checkOperation || session.getNoOperationTime() > sessionTimeout)) {
161167
SessionHandle handle = session.getSessionHandle();
@@ -170,24 +176,34 @@ public void run() {
170176
session.closeExpiredOperations();
171177
}
172178
}
179+
sleepFor(interval);
173180
}
174181
}
175182

176-
private void sleepInterval(long interval) {
177-
try {
178-
Thread.sleep(interval);
179-
} catch (InterruptedException e) {
180-
// ignore
183+
private void sleepFor(long interval) {
184+
synchronized (timeoutCheckerLock) {
185+
try {
186+
timeoutCheckerLock.wait(interval);
187+
} catch (InterruptedException e) {
188+
// Ignore, and break.
189+
}
181190
}
182191
}
183192
};
184193
backgroundOperationPool.execute(timeoutChecker);
185194
}
186195

196+
private void shutdownTimeoutChecker() {
197+
shutdown = true;
198+
synchronized (timeoutCheckerLock) {
199+
timeoutCheckerLock.notify();
200+
}
201+
}
202+
187203
@Override
188204
public synchronized void stop() {
189205
super.stop();
190-
shutdown = true;
206+
shutdownTimeoutChecker();
191207
if (backgroundOperationPool != null) {
192208
backgroundOperationPool.shutdown();
193209
long timeout = hiveConf.getTimeVar(

0 commit comments

Comments
 (0)