You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2017/02/24 01:08:35 UTC
[03/50] [abbrv] hive git commit: HIVE-15917 : incorrect error
handling from BackgroundWork can cause beeline query to hang (Sergey
Shelukhin, reviewed by Siddharth Seth)
HIVE-15917 : incorrect error handling from BackgroundWork can cause beeline query to hang (Sergey Shelukhin, reviewed by Siddharth Seth)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/e49a0742
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/e49a0742
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/e49a0742
Branch: refs/heads/hive-14535
Commit: e49a07426884d8494a37046a227ff4a77cf67f57
Parents: 60a36d1
Author: Sergey Shelukhin <se...@apache.org>
Authored: Thu Feb 16 12:39:55 2017 -0800
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Thu Feb 16 12:39:55 2017 -0800
----------------------------------------------------------------------
.../org/apache/hive/service/cli/CLIService.java | 45 +++++++++++---------
.../hive/service/cli/operation/Operation.java | 16 +------
.../service/cli/operation/SQLOperation.java | 4 +-
3 files changed, 30 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/e49a0742/service/src/java/org/apache/hive/service/cli/CLIService.java
----------------------------------------------------------------------
diff --git a/service/src/java/org/apache/hive/service/cli/CLIService.java b/service/src/java/org/apache/hive/service/cli/CLIService.java
index b842f37..714b259 100644
--- a/service/src/java/org/apache/hive/service/cli/CLIService.java
+++ b/service/src/java/org/apache/hive/service/cli/CLIService.java
@@ -404,7 +404,7 @@ public class CLIService extends CompositeService implements ICLIService {
*/
@Override
public OperationHandle getPrimaryKeys(SessionHandle sessionHandle,
- String catalog, String schema, String table)
+ String catalog, String schema, String table)
throws HiveSQLException {
OperationHandle opHandle = sessionManager.getSession(sessionHandle)
.getPrimaryKeys(catalog, schema, table);
@@ -417,9 +417,9 @@ public class CLIService extends CompositeService implements ICLIService {
*/
@Override
public OperationHandle getCrossReference(SessionHandle sessionHandle,
- String primaryCatalog,
- String primarySchema, String primaryTable, String foreignCatalog,
- String foreignSchema, String foreignTable)
+ String primaryCatalog,
+ String primarySchema, String primaryTable, String foreignCatalog,
+ String foreignSchema, String foreignTable)
throws HiveSQLException {
OperationHandle opHandle = sessionManager.getSession(sessionHandle)
.getCrossReference(primaryCatalog, primarySchema, primaryTable,
@@ -460,6 +460,8 @@ public class CLIService extends CompositeService implements ICLIService {
// The background operation thread was cancelled
LOG.trace(opHandle + ": The background operation was cancelled", e);
} catch (ExecutionException e) {
+ // Note: Hive ops do not use the normal Future failure path, so this will not happen
+ // in case of actual failure; the Future will just be done.
// The background operation thread was aborted
LOG.warn(opHandle + ": The background operation was aborted", e);
} catch (InterruptedException e) {
@@ -473,23 +475,28 @@ public class CLIService extends CompositeService implements ICLIService {
return opStatus;
}
+ private static final long PROGRESS_MAX_WAIT_NS = 30 * 1000000000l;
private JobProgressUpdate progressUpdateLog(boolean isProgressLogRequested, Operation operation) {
- if (isProgressLogRequested && canProvideProgressLog()) {
- if (OperationType.EXECUTE_STATEMENT.equals(operation.getType())) {
- SessionState sessionState = operation.getParentSession().getSessionState();
- try {
- while (sessionState.getProgressMonitor() == null && !operation.isFinished()) {
- Thread.sleep(10L); // sleep for 10 ms
- }
- } catch (InterruptedException e) {
- LOG.warn("Error while getting progress update", e);
- }
- if (sessionState.getProgressMonitor() != null) {
- return new JobProgressUpdate(sessionState.getProgressMonitor());
- }
- }
+ if (!isProgressLogRequested || !canProvideProgressLog()
+ || !OperationType.EXECUTE_STATEMENT.equals(operation.getType())) {
+ return new JobProgressUpdate(ProgressMonitor.NULL);
+ }
+
+ SessionState sessionState = operation.getParentSession().getSessionState();
+ long startTime = System.nanoTime();
+ int timeOutMs = 8;
+ try {
+ while (sessionState.getProgressMonitor() == null && !operation.isDone()) {
+ long remainingMs = (PROGRESS_MAX_WAIT_NS - (System.nanoTime() - startTime)) / 1000000l;
+ if (remainingMs <= 0) return new JobProgressUpdate(ProgressMonitor.NULL);
+ Thread.sleep(Math.min(remainingMs, timeOutMs));
+ timeOutMs <<= 1;
+ }
+ } catch (InterruptedException e) {
+ LOG.warn("Error while getting progress update", e);
}
- return new JobProgressUpdate(ProgressMonitor.NULL);
+ ProgressMonitor pm = sessionState.getProgressMonitor();
+ return new JobProgressUpdate(pm != null ? pm : ProgressMonitor.NULL);
}
private boolean canProvideProgressLog() {
http://git-wip-us.apache.org/repos/asf/hive/blob/e49a0742/service/src/java/org/apache/hive/service/cli/operation/Operation.java
----------------------------------------------------------------------
diff --git a/service/src/java/org/apache/hive/service/cli/operation/Operation.java b/service/src/java/org/apache/hive/service/cli/operation/Operation.java
index 2039946..11a820f 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/Operation.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/Operation.java
@@ -206,20 +206,8 @@ public abstract class Operation {
this.lastAccessTime = System.currentTimeMillis();
}
- public boolean isRunning() {
- return OperationState.RUNNING.equals(state);
- }
-
- public boolean isFinished() {
- return OperationState.FINISHED.equals(state);
- }
-
- public boolean isCanceled() {
- return OperationState.CANCELED.equals(state);
- }
-
- public boolean isFailed() {
- return OperationState.ERROR.equals(state);
+ public boolean isDone() {
+ return state.isTerminal();
}
protected void createOperationLog() {
http://git-wip-us.apache.org/repos/asf/hive/blob/e49a0742/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
----------------------------------------------------------------------
diff --git a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
index 668b4b7..7dde7bf 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
@@ -346,6 +346,7 @@ public class SQLOperation extends ExecuteStatementOperation {
}
runQuery();
} catch (HiveSQLException e) {
+ // TODO: why do we invent our own error path op top of the one from Future.get?
setOperationException(e);
LOG.error("Error running hive query: ", e);
} finally {
@@ -361,8 +362,7 @@ public class SQLOperation extends ExecuteStatementOperation {
} catch (Exception e) {
setOperationException(new HiveSQLException(e));
LOG.error("Error running hive query as user : " + currentUGI.getShortUserName(), e);
- }
- finally {
+ } finally {
/**
* We'll cache the ThreadLocal RawStore object for this background thread for an orderly cleanup
* when this thread is garbage collected later.