Skip to content

Conversation

@Ngone51
Copy link
Member

@Ngone51 Ngone51 commented Aug 12, 2020

What changes were proposed in this pull request?

This PR replaces some arbitrary task names in logs with the widely used task name (e.g. "task 0.0 in stage 1.0 (TID 1)") among driver and executor. This will change the task name in TaskDescription by appending TID.

Why are the changes needed?

Some logs are still using TID(a.k.a taskId) only as the task name, e.g.,

logWarning(s"Thread dump from task $taskId:\n${thread.stackTrace}")

logWarning(s"TID ${taskId} encountered a ${fetchFailedCls} and " +
s"failed, but the ${fetchFailedCls} was hidden by another " +
s"exception. Spark is handling this like a fetch failure and ignoring the " +
s"other exception: $t")

And the task thread name also only has the taskId:

val threadName = s"Executor task launch worker for task $taskId"

As mentioned in #1259, TID itself does not capture stage or retries, making it harder to correlate with the application. It's inconvenient when debugging applications.

Actually, task name like "task name (e.g. "task 0.0 in stage 1.0 (TID 1)")" has already been used widely after #1259. We'd better follow the naming convention.

Does this PR introduce any user-facing change?

Yes. Users will see the more consistent task names in the log.

How was this patch tested?

Manually checked.

@Ngone51
Copy link
Member Author

Ngone51 commented Aug 12, 2020

@jiangxb1987 @tgravescs @squito Please take a look, thanks!

@SparkQA
Copy link

SparkQA commented Aug 12, 2020

Test build #127377 has finished for PR 29418 at commit ade4c9e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@Ngone51
Copy link
Member Author

Ngone51 commented Aug 18, 2020

@jiangxb1987 @tgravescs please help review, thanks!

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

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

LGTM

@jiangxb1987
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Aug 19, 2020

Test build #127603 has finished for PR 29418 at commit ade4c9e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in a1a32d2 Aug 19, 2020
@Ngone51
Copy link
Member Author

Ngone51 commented Aug 19, 2020

thanks all!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants