Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MINOR][Web UI] Correctly convert bytes in web UI #14804

Closed
wants to merge 1 commit into from

Conversation

jerryshao
Copy link
Contributor

What changes were proposed in this pull request?

should be 1024 based, not 1000.

How was this patch tested?

Manually verified.

@srowen
Copy link
Member

srowen commented Aug 25, 2016

KB = 1000 bytes, KiB = 1024 bytes. According to the suffixes we're using, 1000 is correct at the moment. Is the display inconsistent with something else in the UI or logs?

@jerryshao
Copy link
Contributor Author

jerryshao commented Aug 25, 2016

Because in the log it shows Memory MB in 1024 based, while in the web UI it is 1000 based, so this is slightly different.

You could check Utils#bytesToString. I think we should unify this.

@srowen
Copy link
Member

srowen commented Aug 25, 2016

Ugh, yeah that's wrong in the sense that we are not showing MB, but MiB. I'd favor fixing the labels here and in Utils.bytesToString?

Then again, I see that we will also parse input of "500kb" as if it's "500KiB", using 1024 not 1000. That's wrong too really. But fixing it means a bit of a behavior change. We can support "500KB" or "500kb" but it would now mean 500_1000 bytes not 500_1024.

Well, maybe best to be consistently wrong than inconsistently wrong. Anyone feel at all inclined to take the hit in behavior change or just leave it?

@jerryshao
Copy link
Contributor Author

I think here has a precise definition. AFAIK in Spark the conversion is 1024 based either KB, K, or kb, KiB is not so commonly used. And we usually treat everything as 1024 based, so it might not be so necessary to differentiate them.

@srowen
Copy link
Member

srowen commented Aug 25, 2016

Meh, it's still ambiguous and there's a defined way to disambiguate, so it's unfortunate, but I'm OK with a step towards consistency in any event.

@SparkQA
Copy link

SparkQA commented Aug 25, 2016

Test build #64410 has finished for PR 14804 at commit fe78ecc.

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

@SparkQA
Copy link

SparkQA commented Feb 15, 2017

Test build #72944 has finished for PR 14804 at commit fe78ecc.

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

@maropu maropu mentioned this pull request Apr 23, 2017
maropu added a commit to maropu/spark that referenced this pull request Apr 23, 2017
@asfgit asfgit closed this in e9f9715 Apr 24, 2017
peter-toth pushed a commit to peter-toth/spark that referenced this pull request Oct 6, 2018
This pr proposed to close stale PRs. Currently, we have 400+ open PRs and there are some stale PRs whose JIRA tickets have been already closed and whose JIRA tickets does not exist (also, they seem not to be minor issues).

// Open PRs whose JIRA tickets have been already closed
Closes apache#11785
Closes apache#13027
Closes apache#13614
Closes apache#13761
Closes apache#15197
Closes apache#14006
Closes apache#12576
Closes apache#15447
Closes apache#13259
Closes apache#15616
Closes apache#14473
Closes apache#16638
Closes apache#16146
Closes apache#17269
Closes apache#17313
Closes apache#17418
Closes apache#17485
Closes apache#17551
Closes apache#17463
Closes apache#17625

// Open PRs whose JIRA tickets does not exist and they are not minor issues
Closes apache#10739
Closes apache#15193
Closes apache#15344
Closes apache#14804
Closes apache#16993
Closes apache#17040
Closes apache#15180
Closes apache#17238

N/A

Author: Takeshi Yamamuro <yamamuro@apache.org>

Closes apache#17734 from maropu/resolved_pr.

Change-Id: Id2e590aa7283fe5ac01424d30a40df06da6098b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants