Skip to content

[SPARK-29003][CORE] Add start method to ApplicationHistoryProvider to avoid deadlock on startup #25705

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.

Sign up for GitHub

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

Closed
shanyu wants to merge 7 commits into from
Closed

[SPARK-29003][CORE] Add start method to ApplicationHistoryProvider to avoid deadlock on startup #25705

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
624206e
Merge pull request #1 from apache/master
shanyu Sep 6, 2019
c27e35e
SPARK-29003: Spark history server startup hang due to deadlock
shanyu Sep 6, 2019
6fcf84c
Addressing review feedback
shanyu Sep 10, 2019
c3b4411
Addressing feedback
shanyu Sep 10, 2019
046dd76
Fix test failure
shanyu Sep 10, 2019
e8d8024
Add comments to start()
shanyu Sep 12, 2019
75dc86d
Update comments for start()
shanyu Sep 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/src/main/scala/org/apache/spark/deploy/history/ApplicationHistoryProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ private[history] abstract class ApplicationHistoryProvider {
*/
def stop(): Unit = { }

def start(): Unit = { }

@dongjoon-hyun dongjoon-hyun Sep 12, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Could you add a function description like def stop()?

@shanyu shanyu Sep 12, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks!


/**
* Returns configuration data to be shown in the History Server home page.
*
Expand Down
6 changes: 5 additions & 1 deletion core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private[history] class FsHistoryProvider(conf: SparkConf, clock: Clock)
}
}

val initThread = initialize()
var initThread: Thread = null

private[history] def initialize(): Thread = {
if (!isFsInSafeMode()) {
Expand Down Expand Up @@ -384,6 +384,10 @@ private[history] class FsHistoryProvider(conf: SparkConf, clock: Clock)
Map("Event log directory" -> logDir.toString) ++ safeMode
}

override def start(): Unit = {
initThread = initialize()
}

override def stop(): Unit = {
try {
if (initThread != null && initThread.isAlive()) {
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ object HistoryServer extends Logging {

val server = new HistoryServer(conf, provider, securityManager, port)
server.bind()
provider.start()

ShutdownHookManager.addShutdownHook { () => server.stop() }

Expand Down