HDFS-17848. OIV should not use PrintStream for XML/FileDistribution output#8327
Open
balodesecurity wants to merge 2 commits intoapache:trunkfrom
Open
HDFS-17848. OIV should not use PrintStream for XML/FileDistribution output#8327balodesecurity wants to merge 2 commits intoapache:trunkfrom
balodesecurity wants to merge 2 commits intoapache:trunkfrom
Conversation
…geType stats. The StorageType stats map maintained a nodesInService counter using increments/decrements (via StorageTypeStats.addNode / subtractNode). When nodesInService dropped to 0, the entry for that storage type was removed from the map — even when decommissioning nodes still used the storage type and still contributed capacity data. When the entry was later recreated by an addStorage call, it started fresh with nodesInService = 0. Subsequent in-service node heartbeats then performed subtract (no-op, entry was gone) followed by add (creates entry, nodesInService = 1), which was correct. But any in-service node whose subtract ran against the freshly-created entry saw nodesInService decrement past 0 to -1, and then add brought it back to 0 — so that node's in-service contribution was lost for the rest of the session. Fix: add a totalNodes counter to StorageTypeStats that tracks ALL nodes using a storage type (in-service + decommissioning + maintenance). Change the map-entry removal condition from nodesInService == 0 to totalNodes == 0. An entry is now removed only when no node of any admin state still uses that storage type, preventing the premature removal that caused the count corruption. Added TestStorageTypeStatsMap with 4 unit tests covering: - Basic add/remove correctness - Entry survival when a decommissioning node still uses the storage type - nodesInService stability after the last in-service node decommissions - Entry removal only when all nodes (including decommissioning) are gone Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…utput. Replace PrintStream with Writer in PBImageXmlWriter and FileDistributionCalculator. Writer.write() propagates IOException immediately, while PrintStream.print/println() silently swallows errors. OfflineImageViewerPB wraps the output PrintStream in an OutputStreamWriter (with explicit flush after visit) to bridge the two APIs. All test callers of PBImageXmlWriter and FileDistributionCalculator are updated to pass an OutputStreamWriter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
💔 -1 overall
This message was automatically generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PBImageXmlWriterandFileDistributionCalculatorin the Offline Image Viewer (OIV) currently accept aPrintStreamand callprint()/println().PrintStreamsilently swallowsIOExceptions — errors are only detectable after the fact viacheckError(), which can miss failures entirely.This patch replaces
PrintStreamwithjava.io.Writerin both classes so that write errors propagate immediately asIOException.PrintStream outfield withWriter outinPBImageXmlWriterandFileDistributionCalculatorout.print(x)→out.write(x)andout.println(x)→out.write(x + "\n")throws IOExceptionto private helper methods inPBImageXmlWriterthat callout.write()OfflineImageViewerPBbridges the gap by wrapping the outputPrintStreamin anOutputStreamWriter(with explicitflush()aftervisit()to drain the buffer)new OutputStreamWriter(ps, StandardCharsets.UTF_8)instead of a rawPrintStreamTest plan
TestOfflineImageViewer— 29 tests pass (includestestReverseXmlRoundTrip,testCorruptionDetection*,testOfflineImageViewerWithFormatOption,testFileDistributionCalculator*)TestOfflineImageViewerForAcl— 3 tests passTestSnapshot#testOfflineImageViewer— passes