fix(core): format the graph space storage percentage with Locale.ROOT - #3216
Conversation
GraphSpace.info() formats the storage percentage with String.format and
parses it back with Float.parseFloat. Without a Locale the format follows
the JVM default, so on pl_PL, de_DE, fr_FR, ru_RU and other decimal-comma
locales it yields "0,33", which parseFloat rejects: with usePD=true a
fresh PD makes the server exit at startup (Can't write json: For input
string: "0,00"), an existing PD lets it start degraded (some graphs never
load, every drop fails), and GET /graphspaces/{space} answers 400.
Locale.ROOT keeps the arithmetic and the two-digit rounding unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3216 +/- ##
============================================
+ Coverage 41.13% 41.15% +0.01%
- Complexity 7212 7215 +3
============================================
Files 802 802
Lines 69393 69393
Branches 9237 9237
============================================
+ Hits 28546 28556 +10
+ Misses 37583 37580 -3
+ Partials 3264 3257 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified that would block approval.
Pull request overview
Fixes locale-dependent GraphSpace.info() serialization for decimal-comma JVM locales.
Changes:
- Uses
Locale.ROOTfor storage percentage formatting. - Adds locale regression tests and registers them in
UnitTestSuite.
File summaries
| File | Description |
|---|---|
| hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java | Updated as part of this pull request. |
| hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphSpaceInfoLocaleTest.java | Updated as part of this pull request. |
| hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/space/GraphSpace.java | Updated as part of this pull request. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
+1. Review score: 9.5/10. Two independent global reviews and an adversarial review found no actionable issues at f8bdb84. The Locale.ROOT change fixes locale-dependent serialization while preserving the existing arithmetic and rounding.
Locally verified on Java 11: GraphSpaceInfoLocaleTest passes both tests across the six covered locales; the reactor build succeeds. All 24 checks are green on this exact head. No additional code changes are needed. I did not independently rerun the author's full distributed startup experiment.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The production fix is correct, but the new locale regression test can leak category-specific JVM locale state into later tests. Evidence: Locale.setDefault(locale) updates the FORMAT and DISPLAY defaults; the test saves only Locale.getDefault() and restores both categories from that one value.
Purpose of the PR
GraphSpace.info()formats the storage percentage withString.format("%.2f", …)and parses it back withFloat.parseFloat. Without aLocalethe format follows the JVM default locale, so onpl_PL,de_DE,fr_FR,ru_RUand every other decimal-comma locale it yields"0,33", whichparseFloatrejects. Sinceinfo()runs on every serialisation of aGraphSpace, withusePD=truea fresh PD makes the Server exit at startup (Can't write json: For input string: "0,00"), an existing PD lets it start degraded (some graphs never load, every drop fails), andGET /graphspaces/{space}answers 400.Main Changes
GraphSpace.info()formats withLocale.ROOT; the arithmetic and the two-digit rounding are unchanged.unit/core/GraphSpaceInfoLocaleTest(inUnitTestSuite):storage_percentis0.33for 33/100 underpl_PL,de_DE,fr_FR,ru_RUand underLocale.ROOTanden_US; the default locale is restored infinally.Verifying these changes
GraphSpaceInfoLocaleTest2/2 on JDK 11.usePD=true, Server JVM started with-Duser.language=pl -Duser.country=PLagainst a PD that already holds the DEFAULT space): masterfabe0f0b→GET /graphspaces/DEFAULTanswers400 Can't write json: For input string: "0,00"; this PR → 200 with the full space description. Logs inresults/f10-fix/of https://github.com/SebastianGruza/hugegraph-validation.