Skip to content

[SPARK-16650] Improve documentation of spark.task.maxFailures#14287

Closed
tgravescs wants to merge 2 commits intoapache:masterfrom
tgravescs:SPARK-16650
Closed

[SPARK-16650] Improve documentation of spark.task.maxFailures#14287
tgravescs wants to merge 2 commits intoapache:masterfrom
tgravescs:SPARK-16650

Conversation

@tgravescs
Copy link
Contributor

Clarify documentation on spark.task.maxFailures

No tests run as its documentation

@tgravescs
Copy link
Contributor Author

@squito @kayousterhout

<td>
Number of individual task failures before giving up on the job.
Number of failures of any particular task before giving up on the job. The same
task has to fail this number of attempts, it isn't affected by the number of failures
Copy link
Contributor

Choose a reason for hiding this comment

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

for 2nd sentence maybe "Failures spread across different tasks will only cause the job to fail if one particular task has failed this number of times."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see the 2 sentence not being clear, but the second part of your suggestion about "if one particular task has failed" sounds a bit out of place to me, how about:

"The total number of failures spread across different tasks will not cause the job to fail, it has to be a particular task failing this number of attempts."

Copy link
Contributor

Choose a reason for hiding this comment

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

"The total number of failures spread across different tasks will not cause the job to fail; a particular task has to fail this number of attempts."? (just to make it grammatically correct)

Copy link
Contributor

Choose a reason for hiding this comment

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

(or your version is fine too if you don't think the grammar matters; either one is clear enough to the user I think)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I am always open to grammar suggestions. I will update shortly.

@rxin
Copy link
Contributor

rxin commented Jul 20, 2016

I find the description "it isn't affected by the number of failures across different tasks." a little bit confusing.

@tgravescs
Copy link
Contributor Author

@rxin Other suggestions or should I just remove that?

@tgravescs
Copy link
Contributor Author

oops didn't see Kay's suggestion, that sounds better to me.

@SparkQA
Copy link

SparkQA commented Jul 20, 2016

Test build #62615 has finished for PR 14287 at commit 6ce0179.

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

@SparkQA
Copy link

SparkQA commented Jul 20, 2016

Test build #62622 has finished for PR 14287 at commit ffef4ab.

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

@squito
Copy link
Contributor

squito commented Jul 20, 2016

lgtm

asfgit pushed a commit that referenced this pull request Jul 22, 2016
Clarify documentation on spark.task.maxFailures

No tests run as its documentation

Author: Tom Graves <tgraves@yahoo-inc.com>

Closes #14287 from tgravescs/SPARK-16650.

(cherry picked from commit 6c56fff)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@srowen
Copy link
Member

srowen commented Jul 22, 2016

Merged to master/2.0

@asfgit asfgit closed this in 6c56fff Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants