[fix](debug) roll byte sizes up at exact unit boundaries in getByteUint#65778
[fix](debug) roll byte sizes up at exact unit boundaries in getByteUint#65778arpitjain099 wants to merge 1 commit into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
5372342 to
c9a8a24
Compare
|
run buildall |
|
/review |
There was a problem hiding this comment.
Automated review opinion: approve; I found no patch-scoped blocking defect.
Critical checkpoint conclusions:
- Goal and proof: the descending
>=comparisons correctly make exact KB/MB/GB/TB thresholds display as1of the larger unit, and the added direct unit cases exercise every changed comparator. Existing cases continue to cover byte and interior KB/MB/GB values. - Scope and clarity: the change is minimal and focused on the shared formatter plus its unit test.
- Concurrency and lifecycle: no new state, locking, asynchronous work, ownership, or lifecycle behavior is introduced.
- Compatibility and parallel paths: there is no configuration, persistence, storage-format, transaction, FE/BE protocol, or data-write change. All repository consumers use this result as human-readable SHOW/proc/profile/quota/error text; none parses or persists it.
- Correctness and performance: threshold-minus-one, exact-threshold, and threshold-plus-one behavior is continuous at all four boundaries; zero, negative sentinels, and large values retain their prior behavior. The implementation remains constant-time with no new allocation or observability need.
- Tests: the leading
1024Lkeeps all new power expressions inlong, and each power-of-two division yields exactly representable1.0. This review was static-only as required by the review runner; no local build or test was run. CheckStyle was green when reviewed, while FE UT and compile jobs were still queued. - User focus: no additional review focus was provided.
Non-blocking metadata note: the PR intentionally changes user-visible SHOW/profile/quota formatting, so the repository's PR standard calls for a release note instead of None. Please summarize that exact KB/MB/GB/TB thresholds now display in the larger unit (for example, 1.000 TB instead of 1024.000 GB).
TPC-H: Total hot run time: 29392 ms |
TPC-DS: Total hot run time: 177310 ms |
ClickBench: Total hot run time: 25.14 s |
FE UT Coverage ReportIncrement line coverage |
|
hi @arpitjain099 , this modification may change some behavior, if user already use previous format to match something. So I suggest not change it, although it is inconsistent with |
|
Makes sense, thanks for running it down. You're right that the boundary output is load-bearing once anything matches on the current format, and those regression cases make it concrete. The consistency with |
What problem does this PR solve?
getByteUintused strict>for each unit threshold, so a value that lands exactly on a boundary drops to the next smaller unit. A 1 TB data quota shows up as "1024.000 GB", 1 GB as "1024.000 MB", and so on. This is visible in SHOW DATA and in the "data size exceeds quota[...]" error from Database.checkDataSizeQuota, and round quotas like SET DATA QUOTA 1T hit it directly.The sibling method
getUintalready uses>=for the same kind of formatting, so this just makesgetByteUintconsistent with it. Changed the four comparisons to>=so exact multiples of 1024 roll up to the right unit.Release note
None
Check List
Added boundary cases (1 KB, 1 MB, 1 GB, 1 TB) to DebugUtilTest. The existing non-boundary assertions are unchanged.