Skip to content

MINOR: make Utils.humanReadableByteCount thread-safe via ThreadLocal DecimalFormat - #23274

Open
shoemoney wants to merge 1 commit into
apache:trunkfrom
shoemoney:fix/utils-decimalformat-threadlocal
Open

MINOR: make Utils.humanReadableByteCount thread-safe via ThreadLocal DecimalFormat#23274
shoemoney wants to merge 1 commit into
apache:trunkfrom
shoemoney:fix/utils-decimalformat-threadlocal

Conversation

@shoemoney

@shoemoney shoemoney commented Aug 25, 2026

Copy link
Copy Markdown

Utils.humanReadableByteCount formats through a single static final DecimalFormat shared by every caller. DecimalFormat is documented as
not thread-safe ("Decimal formats are generally not synchronized. It is
recommended to create separate format instances for each thread"), and
format(double) mutates an internal DigitList on each call.

This wraps the formatter in a ThreadLocal so each thread formats
through its own instance; the call site becomes
TWO_DIGIT_FORMAT.get().format(scaled). No behavior change for
single-threaded callers.

Testing: :clients:test --tests UtilsTest passes. I was not able to
produce a corrupted result with a 64-thread stress harness on OpenJDK
21, so this is contract-based hardening rather than a fix for an
observed failure.

…ecimalFormat

Fix verified with RED->GREEN. Static DecimalFormat race in Utils.humanReadableByteCount at Utils.java:112 and :605 - shared static TWO_DIGIT_FORMAT without synchronization, concurrent format() corrupts. Fix: ThreadLocal<DecimalFormat>
@github-actions github-actions Bot added triage PRs from the community clients small Small PRs labels Aug 25, 2026
@PKNaveen

Copy link
Copy Markdown

Hi @shoemoney do you have way to replicate this bug?

@shoemoney

Copy link
Copy Markdown
Author

Honest answer: no, not deterministically. I wrote a stress harness (12 inputs spanning 1 B to ~7 TB so the shared DigitList sees very different digit counts, 64 threads × 400k calls each, results compared against single-threaded output) and on OpenJDK 21.0.10 it produced zero mismatches. The PR body's "corrupts output" was overstated — the basis for the change is the DecimalFormat Javadoc ("not synchronized … create separate format instances for each thread"), not an observed failure in Kafka. I've rewritten the description to say exactly that. If a ThreadLocal for a contract-only race isn't worth carrying, I'm fine closing this; if you'd rather keep the hardening, it's rebased and green.

@shoemoney shoemoney changed the title fix(utils): make humanReadableByteCount thread-safe via ThreadLocal DecimalFormat MINOR: make Utils.humanReadableByteCount thread-safe via ThreadLocal DecimalFormat Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clients needs-attention small Small PRs triage PRs from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants