Skip to content

Conversation

@venkata91
Copy link
Contributor

@venkata91 venkata91 commented Jul 29, 2020

What changes were proposed in this pull request?

Just few log lines fixes which are logging the object name instead of the stage IDs

Why are the changes needed?

This would make it easier later for debugging.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Just log messages. Existing tests should be enough

@venkata91
Copy link
Contributor Author

@tgravescs This is just a minor log line changes which I found printing object names instead of the IDs.

@tgravescs
Copy link
Contributor

ok to test

@tgravescs
Copy link
Contributor

what did the log look like before? I thought printing the taskSet should give you something like: TaskSet 2.0. where 2 is the stage id and 0 is the stage attempt id. Now its only printing the stage id without the attempt id?

@SparkQA
Copy link

SparkQA commented Jul 29, 2020

Test build #126776 has finished for PR 29279 at commit 39e22c5.

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

@venkata91
Copy link
Contributor Author

venkata91 commented Jul 29, 2020

Infact its not even printing stageId rather printing the TaskSetManager object name. Do you think instead of printing stageId its better to print name taskSet.name?

For eg:

20/07/29 16:10:04 INFO YarnScheduler: Notifying ExecutorAllocationManager to allocate more executors to schedule the unschedulable task before aborting org.apache.spark.scheduler.TaskSetManager@f031375.

20/07/29 16:10:04 INFO YarnScheduler: Waiting for 120000 ms for completely blacklisted task to be schedulable again before aborting org.apache.spark.scheduler.TaskSetManager@f031375.```

@tgravescs
Copy link
Contributor

what if you explicitly put ${taskSet.toString}

@tgravescs
Copy link
Contributor

oh sorry, I was looking at the wrong thing, those are TaskSetManagers not TaskSet

@venkata91
Copy link
Contributor Author

venkata91 commented Jul 29, 2020

oh sorry, I was looking at the wrong thing, those are TaskSetManagers not TaskSet

Yes thats right. Do you think its better to add toString to `TaskSetManager?

I think ${taskSet.stageId} would be good enough. Though I tried with ${taskSet.name} which prints like this:

20/07/29 18:49:31 INFO YarnScheduler: Waiting for 120000 ms for completely blacklisted task to be schedulable again before aborting TaskSet_0.0.

With stageId:

20/07/29 18:52:44 INFO YarnScheduler: Waiting for 120000 ms for completely blacklisted task to be schedulable again before aborting stage 0.```

@tgravescs
Copy link
Contributor

no, I think this is fine.

@maropu maropu changed the title [SPARK-31418][FOLLOW-UP][MINOR] Fix log messages to print stage id in… [SPARK-31418][CORE][FOLLOW-UP][MINOR] Fix log messages to print stage id instead of the object name Jul 30, 2020
@SparkQA
Copy link

SparkQA commented Jul 31, 2020

Test build #126811 has finished for PR 29279 at commit ad847b1.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Jul 31, 2020

Test build #126848 has finished for PR 29279 at commit ad847b1.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Jul 31, 2020

Test build #126873 has finished for PR 29279 at commit ad847b1.

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

@HyukjinKwon
Copy link
Member

Merged to master.

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