[SPARK-30465][Core]bytesToString util adapts to negative number #27144
Conversation
…he Scheduler Delay Time
|
Can one of the admins verify this patch? |
|
|
@sddyljsx thanks for providing the snapshot. I feel negative bytes may caused by mistakenly computing of memory or overflow. I think it's better to find out the root cause of negative bytes while improving formats for negative bytes doesn't make sense to me. |
It is very necessary to to find out the root cause. I am working on this these days and a new PR will be added. But I think improving formats for negative bytes also makes sense. It's just a general function and no one stipulates that the input of this function cannot be negative. |
|
The case you cite is either a bug, or simply that someone forgot to flip a negative sign or something. I do not think in general there's a situation where negative bytes is useful to display |
|
I also agree with the above comments. I'll close this PR. Thank you, @sddyljsx and all. |
…BlockInfo returns false ### What changes were proposed in this pull request? If `updateBlockInfo` returns false, which means the `BlockManager` will re-register and report all blocks later. So, we may report two times for the same block, which causes `AppStatusListener` to count used memory for two times, too. As a result, the used memory can exceed the total memory. So, this PR changes it to not post `SparkListenerBlockUpdated` when `updateBlockInfo` returns false. And, always clean up used memory whenever `AppStatusListener` receives `SparkListenerBlockManagerAdded`. ### Why are the changes needed? This PR tries to fix negative memory usage in UI (https://user-images.githubusercontent.com/3488126/72131225-95e37e00-33b6-11ea-8708-6e5ed328d1ca.png, see #27144 ). Though, I'm not very sure this is the root cause for #27144 since known information is limited here. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added new tests by xuanyuanking Closes #27306 from Ngone51/fix-possible-negative-memory. Lead-authored-by: yi.wu <yi.wu@databricks.com> Co-authored-by: Yuanjian Li <xyliyuanjian@gmail.com> Co-authored-by: wuyi <yi.wu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…BlockInfo returns false ### What changes were proposed in this pull request? If `updateBlockInfo` returns false, which means the `BlockManager` will re-register and report all blocks later. So, we may report two times for the same block, which causes `AppStatusListener` to count used memory for two times, too. As a result, the used memory can exceed the total memory. So, this PR changes it to not post `SparkListenerBlockUpdated` when `updateBlockInfo` returns false. And, always clean up used memory whenever `AppStatusListener` receives `SparkListenerBlockManagerAdded`. ### Why are the changes needed? This PR tries to fix negative memory usage in UI (https://user-images.githubusercontent.com/3488126/72131225-95e37e00-33b6-11ea-8708-6e5ed328d1ca.png, see #27144 ). Though, I'm not very sure this is the root cause for #27144 since known information is limited here. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added new tests by xuanyuanking Closes #27306 from Ngone51/fix-possible-negative-memory. Lead-authored-by: yi.wu <yi.wu@databricks.com> Co-authored-by: Yuanjian Li <xyliyuanjian@gmail.com> Co-authored-by: wuyi <yi.wu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 30e418a) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
5 participants
What changes were proposed in this pull request?
bytesToString util adapts to negative number
Why are the changes needed?
when the input of bytesToString is negative number, the string returned is unfriendly.
For Example:
-5368709120 will return -5368709120B, not -5.0 GiB
Does this PR introduce any user-facing change?
No
How was this patch tested?
The UT has been added in UtilsSuite