[SPARK-3941][CORE] _remainingmem should not increase twice when updateBlockInfo

[liyezhang556520]

In BlockManagermasterActor, _remainingMem would increase memSize for twice when updateBlockInfo if new storageLevel is invalid and old storageLevel is "useMemory". Also, _remainingMem should increase with original memory size instead of new memSize.

[AmplabJenkins]

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

[liyezhang556520]

Jenkins, retest this please.

[SparkQA]

QA tests have started for PR 2792 at commit 0380a32.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2792 at commit 0380a32.

• 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/21721/
Test PASSed.

[andrewor14]

The old code seems a little more concise

[andrewor14]

Hey @liyezhang556520 I believe your fix is correct. However, if we put it in the isValid case it seems that it's now duplicated in two places. Would it make sense to keep the code you deleted and remove it in the case where it's inValid?

[liyezhang556520]

Good idea, I'll update it soon.

[liyezhang556520]

@andrewor14 , code updated, would you please have a look?

[SparkQA]

QA tests have started for PR 2792 at commit 3d487cc.

• This patch merges cleanly.

[andrewor14]

This seems like a change in semantics. Can you explain why the old code is incorrect (is it)? I'm just trying to understand what this block is trying to do.

[liyezhang556520]

"memSize" is the current memory that the block consume, "originalMemSize" is the memory used before block updating. These two size might be not the same. So here, if the block exists on the slave already, _remainingmem should first _remainingMem += originalMemSize and then _remainingMem -= memSize if the new storage level is "useMemory". Do I have misunderstanding of these two mem size?

[andrewor14]

I see, we want to release the originalMemSize and occupy the new memSize. That makes sense. It seems that the old code is just wrong in some cases then.

[andrewor14]

LGTM. I'm merging this.

[SparkQA]

QA tests have finished for PR 2792 at commit 3d487cc.

• 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/21829/
Test PASSed.