[SPARK-20691][CORE] Difference between Storage Memory as seen internally and in web UI

[mkesselaers]

What changes were proposed in this pull request?

Changed the Strings from MegaBytes to MebiBytes (and likewise for other sizes)

Please review http://spark.apache.org/contributing.html before opening a pull request.

[srowen]

No, this is wrong. The function is still computing megabytes, etc.
Did you see my comment in JIRA? there is more to this change.

[mkesselaers]

@srowen , my apologies, I must have misunderstood you.
I changed the calculation to be MebiBytes, but afterwards saw in the tests that the calculation was already correct
test("bytesToString") { assert(Utils.bytesToString(10) === "10.0 B") assert(Utils.bytesToString(1500) === "1500.0 B") assert(Utils.bytesToString(2000000) === "1953.1 KB") assert(Utils.bytesToString(2097152) === "2.0 MB") assert(Utils.bytesToString(2306867) === "2.2 MB") assert(Utils.bytesToString(5368709120L) === "5.0 GB") assert(Utils.bytesToString(5L * (1L << 40)) === "5.0 TB") assert(Utils.bytesToString(5L * (1L << 50)) === "5.0 PB") assert(Utils.bytesToString(5L * (1L << 60)) === "5.0 EB") assert(Utils.bytesToString(BigInt(1L << 11) * (1L << 60)) === "2.36E+21 B") }

Can you please help me with the one?

[srowen]

Those tests are wrong, in that 2000000 bytes is not "1953.1 KB". It is "1953.1 KiB". And so on. The numbers should stay the same to make sure behavior doesn't actually change, but the labels should be fixed.

The utils.js correctly computed MB, etc, but, it should compute MiB and label it as such, for consistency. This was the original issue.

[mkesselaers]

OK, the tests are fixed to take this into account.
BytesToString, now correctly output "KiB" instead of "KB" and utils.js now also calculates the correct value.

There is still a small difference between BytesToString and FormatBytes, because the first only goes towards a higher size when it is at least 2 MiB, fe.
Should we adapt this as well?

[srowen]

That's looking good. It may be worth searching the code for any other instances of "KB" to see if anything else is formatting numbers inconsistently.

I think this is a safe fix because it is cosmetic.

[mkesselaers]

@srowen ,
I propose to rewrite the variable names of bytesToString as well, to be consistent.
Other occurences of KB can be found in the ivy-report.xsl and in the ByteSuffixes of JavaUtils.
Since there seems to be a lot of logic behind the JavaUtils, I would not change this.

[mkesselaers]

@srowen , I changed the variable names for bytesToString.
As far as I see, this should be everything.

[SparkQA]

Test build #3783 has finished for PR 18229 at commit 71c2bbe.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

@mkesselaers looks like there is at least one more test testing the formatted string that will need to be updated

[mkesselaers]

@srowen, you're right. I changed the strings there as well.

[SparkQA]

Test build #3784 has finished for PR 18229 at commit dd9cc4d.

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

[srowen]

@kishorvpatil @wzhfy do you have any thoughts on this? it touches some code you created

[wzhfy]

I agree with changing the base 1000 to 1024. About MiB and MB, strictly speaking, they are different. But I think in the computing technology world, most people say Megabyte (MB) instead of Mebibytes even though they know the difference. MiB may seem rare for many users.
That's only my personal thoughts, I don't have strong disagreement on this.

[jiangxb1987]

@mkesselaers are you still working on this?

[kiszk]

gentle ping @mkesselaers

[AmplabJenkins]

Can one of the admins verify this patch?

[HyukjinKwon]

gentle ping @mkesselaers