Skip to content
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

[SPARK-21099][Spark Core] INFO Log Message Using Incorrect Executor I… #18308

Closed
wants to merge 3 commits into from
+3 −1
Closed

[SPARK-21099][Spark Core] INFO Log Message Using Incorrect Executor I… #18308

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00a42e7
[SPARK-21099][Spark Core] INFO Log Message Using Incorrect Executor I…
Jun 14, 2017
0f1c467
[SPARK-21099][Core] INFO Log Message Using Incorrect Executor Idle Ti…
Jun 15, 2017
871ed1d
[SPARK-21099][Core] INFO Log Message Using Incorrect Executor Idle Ti…
Jul 2, 2017
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
4 changes: 3 additions & 1 deletion core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,10 @@ private[spark] class ExecutorAllocationManager(
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
newExecutorTotal -= 1
val hasCachedBlocks = SparkEnv.get.blockManager.master.hasCachedBlocks(executorId);
Copy link
Contributor

@jerryshao jerryshao Jun 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This variable executorId is not defined, should change to removedExecutorId.

Copy link
Contributor

Choose a reason for hiding this comment

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

And final semicolon ";" is not necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, there could be a chance when querying executor from BlockManager, the executor/block manager was already removed, so we will potentially get false.

val timeout = if (hasCachedBlocks) cachedExecutorIdleTimeoutS else executorIdleTimeoutS;
logInfo(s"Removing executor $removedExecutorId because it has been idle for " +
s"$executorIdleTimeoutS seconds (new desired total will be $newExecutorTotal)")
s"$timeout seconds (new desired total will be $newExecutorTotal)")
executorsPendingToRemove.add(removedExecutorId)
}
executorsRemoved
Expand Down