[SPARK-5771] Number of Cores in Completed Applications of Standalone Master Web Page always be 0 if sc.stop() is called

[marsishandsome]

In Standalone mode, the number of cores in Completed Applications of the Master Web Page will always be zero, if sc.stop() is called.
But the number will always be right, if sc.stop() is not called.
The reason maybe:
after sc.stop() is called, the function removeExecutor of class ApplicationInfo will be called, thus reduce the variable coresGranted to zero. The variable coresGranted is used to display the number of Cores on the Web Page.

[AmplabJenkins]

Can one of the admins verify this patch?

[srowen]

Might be able to use math.max here. Also should the column heading say "Cores (max)" in the case of completed apps only? Otherwise LGTM.

[jerryshao]

From my understanding, this is an expected behavior since app is finished, number of cores occupied return to ZERO. So you want to display the cores used even when the app is finished, is that right?

[srowen]

I also agree with that interpretation, in which case this column is unuseful for completed apps, and could be removed (or blanked, to retain alignment). If it shows anything it should be max or average or something.

[marsishandsome]

The problem here is that the Cores displayed here is confused, because it depends on whether sc.stop() is called or not.

So I choose to display core max here.

[jerryshao]

I think it really depends on how you interpret this :).

[marsishandsome]

I'm really confused with the core number show above.

[squito]

@jerryshao I agree with @marsishandsome on this one. The issue is not what you should display for # of cores when an app stops -- the issue that it should display something consistent whether or not the app calls sc.stop. (Notice that in the attached img, both apps are completed.)

I don't have any strong feelings whether it should be 0 or max or avg, but I do think it should be consistent whether or not sc.stop was called.

[andrewor14]

Looks good for now. However, in the future if we want to extend dynamic allocation to standalone mode we will have executors coming and going all the time, in which case coresMax will be arbitrarily large and it might make less sense then. @marsishandsome also what happens if executors fail multiple times (but not enough to fail the application), do we count all those cores too?

[marsishandsome]

@andrewor14 What about showing the core number users requested (total-executor-cores) for now?
Users or Administrators (at least me) may want to see this information.

[jerryshao]

Thanks @squito , seems this is really a issue, the problem is that cores might be varied accordingly as executor failed or added (dynamic allocation), coresMax might not be a good choice, what about the initial cores requested myMaxCores and rename the header? Just my thoughts :).

[marsishandsome]

For Running Applications, Cores Using and Cores Requested will be shown.
For Completed Applications, only Cores Requested will be shown.

What do you think about it?

[srowen]

Nit: Cores Using -> Cores In Use

[andrewor14]

more nit: Cores in Use? (lower case i)

[srowen]

FWIW I like the current solution in the screenshot.

[srowen]

See https://issues.apache.org/jira/browse/SPARK-5076 too; it suggests that in addition to removing the current cores column for completed apps, to remove the Memory per Node column too. WDYT?

[andrewor14]

Hey @marsishandsome the new solution looks good. Once you address @srowen's in-line comments I will merge this. As for SPARK-5076, I think having information about the cores and executor memory actually helps the user identify his/her application. I am inclined to close that one as a won't fix and favor this PR.

[andrewor14]

retest this please

[SparkQA]

Test build #27926 has started for PR 4567 at commit 694796e.

• This patch merges cleanly.

[SparkQA]

Test build #27926 has finished for PR 4567 at commit 694796e.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27926/
Test PASSed.

[srowen]

Looks like everything has been addressed and also LGTM. I'll wait a beat for any more feedback but think this can be merged. (@andrewor14 I'll link your suggestion on SPARK-5076 to see what Josh thinks.)

[jerryshao]

Looks like if defaultCores is not set, Spark will choose Int.MaxValue as a requestedCores, which will be displayed in the UI as:

I think this Cores Requested may be a little confused, I think it would be better to set as MAX, what's your opinion, @andrewor14.

Thanks.

[srowen]

@jerryshao Darn, yeah that needs to be fixed. * or ALL seem good to me. It can be displayed if requestedCore == Int.MaxValue. @marsishandsome what do you think about making a small follow on PR for that fix?

[andrewor14]

I think -- might be more appropriate, since the user didn't specify the number of cores to request. * is also fine since there's local[*] with the same semantics (i.e. this app will grab all the cores available to it).

[jerryshao]

I think * is better, let me do a quick fix based on this JIRA.

[marsishandsome]

Thanks @jerryshao

[andrewor14]

Per discussion on #4841 I'm reverting this.