You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by peterableda <gi...@git.apache.org> on 2016/06/08 03:34:24 UTC
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
GitHub user peterableda opened a pull request:
https://github.com/apache/spark/pull/13552
[SPARK-15813] Use past tense for the cancel container request message
## What changes were proposed in this pull request?
Use past tense for the cancel container request message as it is logged after the updated new `Driver requested a total number of $requestedTotal executor(s)` message.
## How was this patch tested?
This is a trivial change
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/peterableda/spark patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13552.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #13552
----
commit 4472e2d6db3f8496ba4164a5dd8380665fded135
Author: Peter Ableda <ab...@gmail.com>
Date: 2016-06-08T03:31:58Z
Use past tense for the cancel container request message
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13552
**[Test build #60393 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60393/consoleFull)** for PR 13552 at commit [`8213c98`](https://github.com/apache/spark/commit/8213c982159aa5755834f2e08f09668b0284c7b3).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66285987
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
The first message is the Spark Driver requesting the spark backend a total number of executors based on dynamic allocation. The cancel message is coming in later because that is when the backend allocator actually figures out what it needs to ask YARN for. They are different components.
In this case we already have the requested number running or pending to be allocated (from YARN) so we actually have to cancel some of those reqeusts. We haven't canceled them yet, we are about to, so it shouldn't be changed to canceled.
If there is a way to update the log messages to be more clear I"m ok with that, otherwise this should be closed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13552
**[Test build #60393 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60393/consoleFull)** for PR 13552 at commit [`8213c98`](https://github.com/apache/spark/commit/8213c982159aa5755834f2e08f09668b0284c7b3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13552
Jenkins test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66289491
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
The first message implies "I need to cancel 5". The second message says "I'm canceling 5". There is no third message that says "I cancelled 5". I don't think the second message is wrong, yes. But you could try to add that third message where appropriate. Or expand the first message to be clearer about the before and after values.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by peterableda <gi...@git.apache.org>.
Github user peterableda commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66220706
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
Maybe an other solution would be to change the `Driver requested a total number of $requestedTotal executor(s).` to `Driver will request a total number of $requestedTotal executor(s).`
This makes more sense:
```
Driver will request a total number of 619 executor(s).
Canceling requests for 4 executor containers
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66251417
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
Maybe update the "Driver requested a total..." message to include what the previous target was then? If I'm right (not entirely sure) then I think it could tell us that the "driver has now requested 614 executors, which differs from previous target of 619".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13552
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60393/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by peterableda <gi...@git.apache.org>.
Github user peterableda commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66217380
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
No it already did. Follow the messages:
16/06/07 18:53:48 INFO yarn.YarnAllocator: Driver requested a total number of 619 executor(s).
16/06/07 18:53:48 INFO yarn.YarnAllocator: Canceling requests for 4 executor containers
16/06/07 18:53:48 INFO yarn.YarnAllocator: Driver requested a total number of 614 executor(s).
16/06/07 18:53:48 INFO yarn.YarnAllocator: Canceling requests for 5 executor containers
So: 619 - 5 = 614
The Cancel msg is coming after the executors are already canceled and we already logged the new requested a total number.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by peterableda <gi...@git.apache.org>.
Github user peterableda commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66251071
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
Ok, this can be only me... :)
"Driver requested a total number of 614 executor(s)". This means to me that the Driver already requested 614 executors. When I see the "Canceling requests for 5 executor containers" message, my interpretation is that it is cancelling 5 executors from the 614 already requested ones which is not true.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13552
**[Test build #60343 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60343/consoleFull)** for PR 13552 at commit [`b6b3721`](https://github.com/apache/spark/commit/b6b3721802625cd0868acf639fc6fdb81b3f055c).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Improve Canceling log message to ma...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/13552
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by peterableda <gi...@git.apache.org>.
Github user peterableda commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66394240
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
Thanks for the explanation, I see what's happening there now. It would be more clear if we would add the target number to the cancel message like this:
`Canceling requests for $numToCancel executor container(s) to have a new desired total $targetNumExecutors executor(s).`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13552
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66304938
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
I really don't think we want more logging here... it's already noisy enough. The current messages are correct, as Sean and Tom have already pointed out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13552
**[Test build #60343 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60343/consoleFull)** for PR 13552 at commit [`b6b3721`](https://github.com/apache/spark/commit/b6b3721802625cd0868acf639fc6fdb81b3f055c).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66209192
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
I don't think that's accurate. It is going to make requests to cancel below right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66236907
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
I don't follow. It says it _now_ wants 614 not 619, so is going to cancel 5. The request happens on line 361 below.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13552
Merged to master/2.0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13552
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60343/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13552
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by peterableda <gi...@git.apache.org>.
Github user peterableda commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66262459
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
We can show the old value of course but it doesn't solve my problem.
16/06/07 18:53:48 INFO yarn.YarnAllocator: Driver requested a total number of 614 executor(s).
16/06/07 18:53:48 INFO yarn.YarnAllocator: Canceling requests for 5 executor containers
It says Driver requested 614 executors (so it supposed to be already happened). But then it says Canceling 5 executors (it is happening now). This is not correct. The 614 executor is less with 5 executors then the previous status update (619).
The root cause is that these two messages are swapped, this would make total sense:
16/06/07 18:53:48 INFO yarn.YarnAllocator: Canceling requests for 5 executor containers
16/06/07 18:53:48 INFO yarn.YarnAllocator: Driver requested a total number of 614 executor(s).
The minimum change to fix this is the one introduced in this pull request, just to reword the Cancel message.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66404015
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
That seems fine to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13552: [SPARK-15813] Use past tense for the cancel conta...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13552#discussion_r66263782
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -353,7 +353,7 @@ private[yarn] class YarnAllocator(
} else if (missing < 0) {
val numToCancel = math.min(numPendingAllocate, -missing)
- logInfo(s"Canceling requests for $numToCancel executor containers")
+ logInfo(s"Canceled requests for $numToCancel executor container(s)")
--- End diff --
But, it is the fact that it is now 614 and not 619 that causes 5 to be cancelled right? If it first says 5 are being cancelled, then says why, is that not out of order?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13552
The line is too long @peterableda but otherwise looks fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13552
Jenkins add to whitelist
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Use past tense for the cancel container re...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13552
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13552: [SPARK-15813] Improve Canceling log message to make it l...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13552
Jenkins retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org