[#1011] feat(tez): Avoid recompute succeeded task.

[zhengchenyu]

What changes were proposed in this pull request?

Avoid recompute succeeded task. Detailed information see #1011
Here only 2.a, 2.b, 2.c is solved. 2.d will not be solved in this PR.

Why are the changes needed?

Fix: #1011

Does this PR introduce any user-facing change?

New config 'rss.avoid.recompute.succeeded.task' was introduced, default value is false. If set to true, we won't recompute the succeed task when the reason of recompute is about node failed.

Note: I suggest change default value to true after running on production cluster some months.

How was this patch tested?

integration test and test in yarn cluster.

[jerqi]

@lifeSo @bin41215 Could you help me review this pr?

[jerqi]

Could we add some documents if we add new config?

[bin41215]

@lifeSo @bin41215 Could you help me review this pr?

ok

[zhengchenyu]

Could we add some documents if we add new config?

@jerqi The config is just a switch. True is enable this function, False is disable this function. It seems no document which is used to describe tez configuration.

[jerqi]

Could we add some documents if we add new config?

@jerqi The config is just a switch. True is enable this function, False is disable this function. It seems no document which is used to describe tez configuration.

We can add it to this document. If the document is too big, we can also separate it.
https://github.com/apache/incubator-uniffle/blob/master/docs/client_guide.md

[zhengchenyu]

Could we add some documents if we add new config?

@jerqi The config is just a switch. True is enable this function, False is disable this function. It seems no document which is used to describe tez configuration.

We can add it to this document. If the document is too big, we can also separate it. https://github.com/apache/incubator-uniffle/blob/master/docs/client_guide.md

I doubt that whether this config should be introduced to Common Setting in client_guide.md or not.
Because I think avoid recompute succeeded task attempt should be a default function for remote shuffle service.
Why do I introduce this config? The reason is that this function has not run in production for enough months. So I prefer disable in default. After several month, I prefer enable in default or delete this config.

[jerqi]

Could we add some documents if we add new config?

@jerqi The config is just a switch. True is enable this function, False is disable this function. It seems no document which is used to describe tez configuration.

We can add it to this document. If the document is too big, we can also separate it. https://github.com/apache/incubator-uniffle/blob/master/docs/client_guide.md

I doubt that whether this config should be introduced to Common Setting in client_guide.md or not. Because I think avoid recompute succeeded task attempt should be a default function for remote shuffle service. Why do I introduce this config? The reason is that this function has not run in production for enough months. So I prefer disable in default. After several month, I prefer enable in default or delete this config.

This document describe all the config options which we used. You can see there are many
dedicated Spark‘s config options. If we have time, we can separate this document into common_client.md, spark_client.md, mr_client.md, tez_client.md.

[codecov-commenter]

Codecov Report

Merging #1033 (5fdda17) into master (ecfed5e) will increase coverage by 1.37%.
Report is 2 commits behind head on master.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #1033      +/-   ##
============================================
+ Coverage     54.10%   55.47%   +1.37%     
- Complexity     2547     2550       +3     
============================================
  Files           382      362      -20     
  Lines         21728    19405    -2323     
  Branches       1802     1809       +7     
============================================
- Hits          11755    10765     -990     
+ Misses         9269     8011    -1258     
+ Partials        704      629      -75     

Files Changed	Coverage Δ
.../main/java/org/apache/tez/common/RssTezConfig.java	92.85% <ø> (ø)
...n/java/org/apache/tez/dag/app/RssDAGAppMaster.java	26.80% <0.00%> (-3.63%)	⬇️

... and 32 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jerqi]

LGTM, merged to master. Thanks @zhengchenyu @bin41215