Repository navigation
Fix LOAD splitting of sparse batched aligned chunks (complete #19032) - #19184
Open
luoluoyuyu wants to merge 4 commits into
Open
luoluoyuyu wants to merge 4 commits into
luoluoyuyu wants to merge 4 commits into
Conversation
The LOAD splitter re-encodes the standalone value chunks of a batched aligned table with a ValueChunkWriter, and the chunks it builds have to keep the row positions and the page positions of the time chunks they belong to. - Write every row, nulls included, with its real timestamp, so that the value page bitmap keeps one position per row and the statistics cover the non-null values only. - Keep one page per page slot of the time chunk, even when the page is all null: the aligned reader pairs the pages of a value chunk with the pages of its time chunk by position, so a missing page shifts every later page onto the timestamps of an earlier one. - Normalize the header of an empty page. The page writer decides on the presence of the page statistics instead of the page size, so the statistics carried by the placeholder page of an empty chunk would be written into a page that has no data and the reader would parse a phantom page out of it. - Choose between the mirrored layout and the zero-page layout of an all-null column with one rule - the counterpart of the rule the base class applies to the chunks it writes itself - instead of conflating "has a non-null value" with "has rows of this partition". Cover the cases the previous tests missed: a copied page that carries data followed by a decoded page, an all-null page behind the pages that carry data, and an assertion that every emitted chunk declares exactly the number of bytes it carries. Based on the branch of apache#19032.
The LOAD splitter re-encodes the value chunks of a batched aligned chunk group when the file crosses time partitions. The new IT builds such a file with the compaction layout - one time chunk and two value chunks, the second one behind the time chunk - whose rows include null values and an entirely null page, loads it, and checks that every row keeps its timestamp and its value. The file spans three time partitions of the interval the test configures, so LOAD decodes and splits it instead of storing it as it is.
Caideyipi
reviewed
Nov 9, 2026
PageBuffer.satisfiedLength already holds the number of rows the page contributes to the time partition, so the layout decision no longer scans the decoded timestamps a second time.
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.
Fix LOAD splitting of sparse batched aligned chunks (completes #19032)
TL;DR
BatchedAlignedValueChunkDatarebuilds the standalone value chunks of a chunk group when LOADsplits a TsFile by time partition. The old implementation assumed compact data and compact page
slots. For sparse data — which is exactly the layout the compaction writer produces for aligned
tables — it dropped null rows, dropped all-null pages, and could emit a page stream the reader
cannot pair with its time chunk. The visible result is wrong values, lost rows, or a split TsFile
that is silently truncated / cannot be parsed back.
The fix keeps three invariants while re-encoding:
0varint the format defines.Contents
1. Background
1.1 When LOAD re-encodes a chunk
LoadSingleTsFileNode#needDecodeTsFiledecides that a TsFile must be split when a device spansmore than one time partition slot.
TsFileSplitterthen decodes the file and writes one newTsFile per partition. For a chunk that lies inside a single partition the raw bytes are copied
(
writeEntirePage); for a chunk (or page) that crosses a boundary the points are decoded andre-encoded (
writeDecodePage/writeDecodeValuePage).1.2 The layout the compaction writer produces
To keep compaction memory bounded, the writer emits the first series batch together with the time
chunk and appends the remaining value chunks after all time chunks of the chunk group. That is
the "batched aligned" layout, and those trailing chunks are the ones this class rebuilds:
AlignedChunkDatahandles thef1style withAlignedChunkWriterImpl(time and value chunksinterleaved).
f2/f3have no following time chunk, so the splitter cannot use that writer andBatchedAlignedValueChunkDatadrives a plainValueChunkWriterinstead. Both paths must producechunks that the aligned reader accepts.
1.3 Why re-encoding is delicate: the reader's three rules
Rule A — the value page bitmap has one position per timestamp, nulls included
A value page body is:
ValuePageWriter#setBit(isNull)only raises the bit for a non-null value, butsizeisincremented for every row. The reader (
ValuePageReader#splitDataToBitmapAndValue) indexes thebitmap with
timeIndex, i.e. with the position inside the time page.Rule B — page k of a value chunk pairs with page k of its time chunk
AbstractAlignedChunkReader#initAllPageReadersdrives the loop from the time chunk and takesexactly one page from every value chunk per iteration:
The page count also changes how the chunk is parsed:
ChunkHeadersetsnumOfPages <= 1 ? ONLY_ONE_PAGE_CHUNK_HEADER : CHUNK_HEADER, and the reader has a separatebranch for single-page chunks. Declared page count and real page count must agree exactly.
Rule C — an all-null page is exactly one byte
The format's "this page is all null" page is a zero-length body whose header is the single varint
uncompressedSize = 0.PageHeader#deserializeFromreturns immediately in that case, leaving theremaining bytes for the next page:
Any extra byte turns into a phantom page and shifts every following page slot.
2. Problems in master
All of the following are in
BatchedAlignedValueChunkData#encodeAndBuildChunksbefore this PR.P1 — null rows were not written, so the bitmap collapsed
Consequence — with
v(t) = t + 1, null at odd timestamps, one page in partition[0, I-1]:Every value after the first null is attached to a previous timestamp, and the tail of the page
becomes null. This is the "wrong values / missing rows" symptom.
P2 — all-null pages were not written, so every later page shifted
Two different code paths dropped a page slot:
Not sealing is worse than "dropping a page":
ValuePageWriteris not reset either, so the nextsource page's rows are appended to the same page writer and the page boundaries are lost as well.
Consequence — partition
[0, I-1]with source pages{0,1}(all null) and{I-2, I-1}(values):P3 — the chunk-level decision conflated "wrote data" with "has rows"
hasBufferedDatawas used to answer two different questions: did the writer buffer anythingand does this chunk cover any row of this partition. For a partition whose pages are
[data][all null][data]the first question is true, so the chunk is written with pages — but theall-null page was skipped (P2), producing 2 pages against 3 time pages:
Conversely, a chunk that has no row at all in the partition must keep the zero-page layout of
an all-null column — and must still be emitted: the measurements of a chunk group are aligned by
chunk position, so dropping the chunk entirely shifts every later measurement.
P4 —
valueBatchmay be shorter thantimeBatchP5 — statistics were patched by hand around a synthetic timestamp
The dummy time
0was compensated afterwards:This mutates the writer's internal page statistics, which the writer itself manipulates for its
"first page statistics are stored only at chunk level" optimization, and it is executed only for
pages that had at least one non-null value — i.e. exactly the pages that P1/P2 were about to
handle incorrectly.
3. The fix
3.1 Write every row, with its real timestamp (fixes P1, P5)
ValuePageWriter#writecallssetBit(isNull)for every row (so the bitmap keeps one position pertimestamp) and
statistics.update(time, value)only for non-null values (so page and chunkstatistics are computed by the writer, exactly as in the normal write path). The manual
setStartTime/setEndTimeblock disappears.Also note the writer already understands all-null pages:
sealCurrentPageseals whenpageWriter.getSize() != 0(positions, nulls included), andValuePageWriter#writePageHeaderAndDataIntoBuffemitswriteEmptyPageIntoBuff— one0byte —when
statistics.getCount() == 0. The format supported all-null pages all along; the splitter wassimply not producing them.
3.2 Seal every page that has rows (fixes P2)
For a copied page, an empty page no longer disappears; it is re-emitted and keeps its slot:
EMPTY_PAGE_HEADERisnew PageHeader(0, 0, null). The header must not carry statistics.This is easy to get wrong:
TsFileSplitter#handleEmptyValueChunkbuilds the placeholder page asnew PageHeader(0, 0, chunkMetadata.getStatistics()), and for the first page of a chunkValueChunkWriter#writePageHeaderAndDataIntoBuff(data, header)writes acompressedSizevarint inaddition to
uncompressedSizewheneverheader.getStatistics() != null, deferring thosestatistics into
firstPageStatisticsto splice them into the buffer later. Re-emitting theplaceholder header as the first page of a chunk therefore produces two bytes where the format
defines one, and the reader parses a phantom second empty page — the real data page then has no
slot and the column silently reads as all null. That is why the fix normalizes the header instead
of reusing it.
3.3 Decide per chunk: mirror the page slots or keep the zero-page layout (fixes P3)
The coarse
hasBufferedDataflag is replaced by a predicate that answers the actual question —does any page of this chunk hold rows of this partition?
Both page kinds answer the question from data the splitter has already recorded, so no timestamp is
visited a second time:
PageBuffer.satisfiedLengthis the number of rows the page contributes tothe partition, filled in by
TsFileSplitterwhile it splits the page(
writeDecodePage(times, values, satisfiedLength)), and a copied page was read from this partitionas a whole, so its body is non-empty exactly when it holds rows. This is the same test the base
class uses in
AlignedChunkData#hasPageWithData.The result selects between the two layouts the reader understands:
This is the counterpart of the rule the base class already applies in
AlignedChunkData#buildChunkFromUndecodedPages(which returnsnulland falls back to the writerpath when a chunk consists only of empty pages). Before this change, the fast path obeyed the
rules and the writer path — the one taken as soon as a single page must be decoded — did not.
3.4 The resulting loop
Change map
BatchedAlignedValueChunkDataconstantsEMPTY_PAGE_DATA,EMPTY_PAGE_HEADERencodeAndBuildChunks()chunkHasRowsreplaceshasBufferedDatapageHasRows, bounds guardpageStartTime/pageEndTime/ statistics patchdataSize/pageCountfromchunkHasRowshasPageWithRows(...)No format change, no change to the normal write/compaction path, no config, no public API.
4. Tests
4.1
TsFileSplitterTest— five new cases, one shared harnessassertSparseBatchedAlignedChunkSplit(pages, isNull)builds the compaction layout withTsFileIOWriter(time chunk +f1throughAlignedChunkWriterImpl, thenf2through astandalone
ValueChunkWriter), verifies the source file reads back as built, runs the realsplitter, then for every partition writes the target TsFile and reads it back row by row.
Besides the row assertions, the harness checks that every chunk declares exactly the bytes it
carries:
...PreservesNullRows...PreservesCopiedEmptyPage{0,1}all null (copied) + a page crossing the boundary...PreservesDecodedEmptyPage...PreservesCopiedPageBeforeDecodedPage...PreservesEmptyPageAfterDataPages[data][all null][data]4.2
IoTDBLoadTsFileIT#testLoadSparseBatchedAlignedTsFileBuilds the same layout as a real file whose three pages span three configured partitions — one
page inside a partition (copied), one crossing a boundary with a null row (decoded), one entirely
null (empty page slot) — then goes through SQL only:
Every row's timestamp,
f1andf2is asserted, includingresultSet.wasNull()for the nullrows. This is the only test that covers LOAD's scheduling, the partition configuration and the
metadata index of the produced files.
5. Validation
mvn spotless:check -pl iotdb-core/datanode(and the same forintegration-testwith-P with-integration-tests): BUILD SUCCESS.mvn validate -pl integration-test -P with-integration-tests: 0 Checkstyle violations.TsFileReaderreturns the expected rows with the expected nulls (this is how a missingendFile()in the fixture was caught).test sources (stale
~/.m2artifacts) and nothriftcompiler for-ambuilds, so neitherTsFileSplitterTestnor the IT can be started here; the changed files were type-checked withjavacagainst the module classpath instead.testCompileofiotdb-serveronWALNodeTest.java:366(cannot find symbol: walFlushListeners), which is a defect introduced onmasterby Fix flaky WALNode outdated file deletion test #17921 and fixed there by Fix WALNodeTest compilation after explicit WAL rolling #19189. That failure is unrelated to this PR and happenedbefore any test of this change could run; the branch has been updated with
master.6. Scope and risk
encodeAndBuildChunks()override of one class, usedexclusively by
TsFileSplitterwhen LOAD has to re-encode a standalone value chunk.AlignedChunkDatabehaviour, configuration or public API.
page count and byte count they contain, and every page slot of the time chunk has a counterpart.
A file that used to be produced with shifted pages now fails loudly (header/data mismatch) or
reads correctly, instead of returning wrong values.
This PR:
unfamiliar reader.
Key changed/added classes
org.apache.iotdb.db.storageengine.load.splitter.BatchedAlignedValueChunkDataorg.apache.iotdb.db.storageengine.load.TsFileSplitterTestorg.apache.iotdb.relational.it.db.it.IoTDBLoadTsFileIT