[common] Grow from one byte in MemorySliceOutput when the segment is empty - #9523
Open
LuciferYang wants to merge 1 commit into
Open
[common] Grow from one byte in MemorySliceOutput when the segment is empty#9523LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
…empty ensureSize doubled the capacity starting from segment.size(), and doubling never leaves zero, so a MemorySliceOutput created with capacity 0 spun on the CPU inside the growth loop on its first write and never returned. The doubling now starts from one byte. Positive capacities take exactly the path they did before. Every construction site in the repository passes a positive size, so no current caller reaches this. Assisted-by: GLM-5.3
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
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.
Purpose
close #9522
MemorySliceOutput.ensureSizegrows the buffer by doubling from the current segment size, and doubling never leaves zero, so aMemorySliceOutputcreated with capacity 0 spun on the CPU inside the growth loop on its first write and never returned. It looks like a hang rather than a bug: a live thread insideensureSize, no exception and no progress. The doubling now starts from one byte, and positive capacities take exactly the path they did before.Every construction site in the repository passes a positive size, the smallest being 2 in the global-index key serializer, so no current Paimon code path reaches this. The class is public in paimon-common and takes its capacity from the caller.
Two older problems in the same loop are left alone:
newCapacity <<= 1overflows above 2^30 and walks through negative back into 0, andsegment.size() + minWritableBytescan overflow so that the loop is skipped andensureSizereturns as though it had grown. Both needlongarithmetic and a clamp against a maximum array size, which turns a hang into a thrown exception for very large requests. That is a separate decision from this one, and it is recorded in #9522.Tests
MemorySliceOutputTestis new; the class had no test before.testWriteAfterZeroInitialCapacitywrites a byte and then three bytes into an output created with capacity 0, and asserts the buffer holds5 1 2 3. The write runs on a separate daemon thread joined for ten seconds, and the test asserts the thread finished: JUnit's@Timeoutin its default same-thread mode only reports after the method returns, so it cannot fail a CPU-bound loop. The thread captures anyThrowableinto anAtomicReferencethat is asserted null before the content assertions, so a future failure inside the write is reported as itself rather than as a wrong buffer length. The daemon flag matters for the same reason the test exists: on a real regression the spinning thread must not outlive the test and hold a core in the surefire fork.Verified red before the change: the join times out and the test fails with
[write did not terminate]after 10.06 seconds.mvn -pl paimon-common teston JDK 8: 12466 tests, 0 failures, 0 errors. checkstyle, spotless, enforcer and rat run clean.