[
https://issues.apache.org/jira/browse/HIVE-26903?focusedWorklogId=837486&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837486
]
ASF GitHub Bot logged work on HIVE-26903:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 06/Jan/23 13:42
Start Date: 06/Jan/23 13:42
Worklog Time Spent: 10m
Work Description: kasakrisz commented on code in PR #3916:
URL: https://github.com/apache/hive/pull/3916#discussion_r1063437196
##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java:
##########
@@ -86,12 +93,18 @@ public Configuration getConf() {
public void init(AtomicBoolean stop) throws Exception {
setPriority(MIN_PRIORITY);
- setDaemon(true); // this means the process will exit without waiting for
this thread
+ setDaemon(false);
this.stop = stop;
this.hostName = ServerUtils.hostname();
this.runtimeVersion = getRuntimeVersion();
}
+ protected void checkInterrupt() throws InterruptedException {
+ if (Thread.interrupted()) {
+ throw new InterruptedException(type.name() + " execution is
interrupted.");
Review Comment:
Since we already have class hierarchy to define common and specific parts of
the code the `CompactorThreadType` enum is not necessary. Especially if it is
used only for naming subclasses.
I think a method like
```
abstract String getName();
```
and the corresponding implementations in each subclass can be used.
Or maybe something like `this.getClass().getName()` also works.
##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java:
##########
@@ -271,7 +272,7 @@ protected Boolean findNextCompactionAndExecute(boolean
collectGenericStats, bool
if (ci == null) {
return false;
}
- if ((runtimeVersion != null || ci.initiatorVersion != null) &&
!runtimeVersion.equals(ci.initiatorVersion)) {
+ if ((runtimeVersion != null && ci.initiatorVersion != null) &&
!runtimeVersion.equals(ci.initiatorVersion)) {
Review Comment:
What happens if `runtimeVersion` is not null but `ci.initiatorVersion` is
null? Shouldn't we log the mismatch in this case?
Issue Time Tracking
-------------------
Worklog Id: (was: 837486)
Time Spent: 40m (was: 0.5h)
> Compactor threads should gracefully shutdown
> --------------------------------------------
>
> Key: HIVE-26903
> URL: https://issues.apache.org/jira/browse/HIVE-26903
> Project: Hive
> Issue Type: Improvement
> Reporter: László Végh
> Assignee: László Végh
> Priority: Major
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently the compactor threads are daemon threads, which means the JVM will
> not wait for these threads to finish. (see:
> [https://github.com/apache/hive/blob/431e7d9e5431a808106d8db81e11aea74f040da5/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java#L81)]
> As a result during system shutdown, JVM may close all daemon threads
> abruptly (JVM won't wait for a thread to sleep/wait, and no
> InterruptedException is thrown), so the threads don't have any chance to
> shutdown gracefully. This can lead to inconsistent/corrupted state in the
> Metastore or on the File system.
> Make the compactor threads user threads, and handle shutdown accordingly.
> Make sure interrupts and InterruptedException is handled accordingly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
