Skip to content

[common] Grow from one byte in MemorySliceOutput when the segment is empty - #9523

Open
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/memory-slice-output-zero-capacity
Open

[common] Grow from one byte in MemorySliceOutput when the segment is empty#9523
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/memory-slice-output-zero-capacity

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Purpose

close #9522

MemorySliceOutput.ensureSize grows the buffer by doubling from the current 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. It looks like a hang rather than a bug: a live thread inside ensureSize, 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 <<= 1 overflows above 2^30 and walks through negative back into 0, and segment.size() + minWritableBytes can overflow so that the loop is skipped and ensureSize returns as though it had grown. Both need long arithmetic 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

MemorySliceOutputTest is new; the class had no test before.

testWriteAfterZeroInitialCapacity writes a byte and then three bytes into an output created with capacity 0, and asserts the buffer holds 5 1 2 3. The write runs on a separate daemon thread joined for ten seconds, and the test asserts the thread finished: JUnit's @Timeout in its default same-thread mode only reports after the method returns, so it cannot fail a CPU-bound loop. The thread captures any Throwable into an AtomicReference that 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 test on JDK 8: 12466 tests, 0 failures, 0 errors. checkstyle, spotless, enforcer and rat run clean.

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant