-
Notifications
You must be signed in to change notification settings - Fork 618
[LIVY-786] Interrupt interpreter threads when cancelling statements #495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ package org.apache.livy.repl | |
|
|
||
| import java.util.{LinkedHashMap => JLinkedHashMap} | ||
| import java.util.Map.Entry | ||
| import java.util.concurrent.Executors | ||
| import java.util.concurrent.{ConcurrentHashMap, Executors} | ||
| import java.util.concurrent.atomic.AtomicInteger | ||
|
|
||
| import scala.collection.JavaConverters._ | ||
|
|
@@ -63,6 +63,8 @@ class Session( | |
| private val cancelExecutor = ExecutionContext.fromExecutorService( | ||
| Executors.newSingleThreadExecutor()) | ||
|
|
||
| private val statementThreads = new ConcurrentHashMap[Int, Thread]() | ||
|
|
||
| private implicit val formats = DefaultFormats | ||
|
|
||
| private var _state: SessionState = SessionState.NotStarted | ||
|
|
@@ -161,18 +163,29 @@ class Session( | |
| _statements.synchronized { _statements(statementId) = statement } | ||
|
|
||
| Future { | ||
| setJobGroup(tpe, statementId) | ||
| statement.compareAndTransit(StatementState.Waiting, StatementState.Running) | ||
| val currentThread = Thread.currentThread() | ||
| statementThreads.put(statementId, currentThread) | ||
| try { | ||
| setJobGroup(tpe, statementId) | ||
| statement.compareAndTransit(StatementState.Waiting, StatementState.Running) | ||
|
|
||
| if (statement.state.get() == StatementState.Running) { | ||
| statement.started = System.currentTimeMillis() | ||
| statement.output = executeCode(interpreter(tpe), statementId, code) | ||
| } | ||
| if (statement.state.get() == StatementState.Running) { | ||
| statement.started = System.currentTimeMillis() | ||
| statement.output = executeCode(interpreter(tpe), statementId, code) | ||
| } | ||
|
|
||
| statement.compareAndTransit(StatementState.Running, StatementState.Available) | ||
| statement.compareAndTransit(StatementState.Cancelling, StatementState.Cancelled) | ||
| statement.updateProgress(1.0) | ||
| statement.completed = System.currentTimeMillis() | ||
| statement.compareAndTransit(StatementState.Running, StatementState.Available) | ||
| statement.compareAndTransit(StatementState.Cancelling, StatementState.Cancelled) | ||
| statement.updateProgress(1.0) | ||
| statement.completed = System.currentTimeMillis() | ||
| } finally { | ||
| statementThreads.remove(statementId, currentThread) | ||
ArnavBalyan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Clear the interrupt flag, but log if the thread was interrupted. | ||
| if (Thread.interrupted()) { | ||
| warn(s"Thread was interrupted during execution of statement $statementId; " + | ||
| "interrupt flag cleared.") | ||
| } | ||
| } | ||
| }(interpreterExecutor) | ||
|
|
||
| statementId | ||
|
|
@@ -212,6 +225,7 @@ class Session( | |
| info(s"Failed to cancel statement $statementId.") | ||
| statement.compareAndTransit(StatementState.Cancelling, StatementState.Cancelled) | ||
| } else { | ||
| Option(statementThreads.get(statementId)).foreach(_.interrupt()) | ||
ArnavBalyan marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ArnavBalyan - Can you please verify that this change is only intended to interrupt interruptible tasks such as sleep, object.wait, etc? As @gyogal has mentioned this will not interrupt actual long running threads. I am also curious about the fact that this is being called within the while loop. Are we waiting for the state to be successfully set to Cancelled instead of still Cancelling? The upon failure, we set it to Cancelled after timing out therefore it will not try to be interrupted again? |
||
| sc.cancelJobGroup(statementId.toString) | ||
| if (statement.state.get() == StatementState.Cancelling) { | ||
| Thread.sleep(livyConf.getTimeAsMs(RSCConf.Entry.JOB_CANCEL_TRIGGER_INTERVAL)) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.