executor: normalize IEC binary unit display for config values#65350
executor: normalize IEC binary unit display for config values#65350majiayu000 wants to merge 2 commits intopingcap:masterfrom
Conversation
When displaying configuration values via SHOW CONFIG, byte size
values like "24MiB" are now normalized to "24MB" for consistency
with the input format. This fixes the mismatch where users input
"24MB" but see "24MiB" in the output.
The docker/go-units library parses both "MB" and "MiB" as binary
units (1024-based) but outputs using IEC notation ("MiB"). This
change normalizes the display to use SI notation ("MB") to match
user expectations.
Fixes pingcap#51740
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: majiayu000 <1835304752@qq.com>
|
Hi @majiayu000. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Welcome @majiayu000! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @majiayu000. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test all |
|
@majiayu000: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1 similar comment
|
@majiayu000: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@pantheon-bot, can you comprehensively review this pr? |
|
I think we should keep the original input from user? Not normalize them to |
|
@lance6716 Thanks for the suggestion about keeping original user input. I implemented raw-size preservation at the source components and opened PRs:\n- tikv/tikv#19257\n- tikv/pd#10126\n- pingcap/tidb-engine-ext#450 (tiflash-proxy)\n\nThese keep the original size string in /config when the parsed bytes match the current value (so we don't show mismatched units). Does this align with your expectation? Any other components you'd like to cover (e.g. TiProxy)? |
|
/ok-to-test |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #65350 +/- ##
================================================
+ Coverage 70.7729% 77.0754% +6.3024%
================================================
Files 1902 1914 +12
Lines 518934 537702 +18768
================================================
+ Hits 367265 414436 +47171
+ Misses 127126 122269 -4857
+ Partials 24543 997 -23546
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Closing this PR due to extended inactivity. Happy to reopen if there's still interest — feel free to comment. |
What problem does this PR solve?
Issue Number: close #51740
Problem Summary:
When displaying configuration values via
SHOW CONFIG, byte size values like "24MiB" were not matching the user's input format "24MB". This caused confusion as users expected to see the same unit notation they used when setting config values.What changed and how does it work?
Added a
normalizeByteSizeUnitfunction that converts IEC binary unit suffixes (MiB, GiB, KiB, TiB, PiB, EiB) to SI decimal unit suffixes (MB, GB, KB, TB, PB, EB) for display consistency.The docker/go-units library parses both "MB" and "MiB" as binary units (1024-based) but outputs using IEC notation ("MiB"). This change normalizes the display to use SI notation ("MB") to match user expectations.
Check List
Tests
Side effects
Documentation
Release note