[SPARK-20391][Core] Rename memory related fields in ExecutorSummay

[jerryshao]

What changes were proposed in this pull request?

This is a follow-up of #14617 to make the name of memory related fields more meaningful.

Here for the backward compatibility, I didn't change maxMemory and memoryUsed fields.

How was this patch tested?

Existing UT and local verification.

CC @squito and @tgravescs .

[SparkQA]

Test build #75978 has finished for PR 17700 at commit 135e233.

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

[squito]

Thanks for doing this quickly @jerryshao. I wasn't very clear about this on the jira, but my original idea was to add another level of nesting in the json, and put all the new memory related metrics under there (leaving maxmemory and memoryUsed as they were for compatibility), eg.:

{
  "id" : "1",
  "hostPort" : "172.22.0.167:51490",
   ...
  "memoryMetrics": {
    "usedOnHeapStorageMemory": ...,
    "usedOffHeapStorageMemory": ...,
    ...
  }
}

I think that organization would be a little more clear.

@tgravescs wdyt?

[tgravescs]

Yes that is what I was thinking from the conversation in the jira. We should do that now as to not cause more compatibility issues.

[jerryshao]

Thanks @squito for clarification, sorry I misunderstood it.

Regarding this new memoryMetrics, will all the memory related metrics be shown here, like what you mentioned in the JIRA?

Also this PR #17625 exposes the Netty memory usage, should this memory usage also be shown here? I'm thinking of a extensible and reasonable place to track all the memory usages, so that our follow-up work will not break this API again.

[SparkQA]

Test build #76015 has started for PR 17700 at commit e1d618b.

[squito]

Jenkins, retest this please

[squito]

lgtm pending tests (and one thing I just want to double check)

[squito]

I just want to double check -- there won't be any issues with event logs from spark 2.1, and only some of these fields being present, right? taking a quick look at 2.1 and your last change #14617, this looks right.

[jerryshao]

Yes, this is to make sure memoryMetrics field will only be existed when replaying new event logs, for Spark 2.1- event log, this field will not be present.

[SparkQA]

Test build #76040 has finished for PR 17700 at commit e1d618b.

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

[jerryshao]

@squito , do you have any further comment? Thanks!

[squito]

thanks for the reminder, sorry it took me a couple days to circle back.

merged to master and 2.2

And thanks a lot for this issue -- for taking another look at these after that last patch, realizing it was not ideal, then working quickly to get a fix in!