Fix sequence space overlap caused by out-of-order tablets by ColinLeeo · Pull Request #19035 · apache/iotdb

ColinLeeo · 2026-10-28T03:15:47Z

Problem

Fixes #19020.

A tablet whose rows are not in ascending time order could write a row older than the device's last flush time into the sequence space. The resulting sequence TsFile overlapped the existing sequence TsFile of the same device, and plain SELECT queries then returned duplicated rows and broke the time order, silently — no exception, no log entry.

A pure-SQL reproduction (create the layout, insert an out-of-order tablet through INSERT INTO ... SELECT, then observe the wrong order) is in the issue.

Root cause

INSERT INTO ... SELECT copies the rows of the child operator verbatim into the insert tablet, while the storage layer documents that it assumes the batch has been sorted by the client (InsertTabletNode.checkTTLInternal). The rows of a SELECT are not ordered — a join, an ORDER BY time DESC or a scan over several devices can emit them in any order — so the storage-side steps below see rows that go backwards in time:

Assumption	Where
the sequence flag only has to be re-evaluated when it is still `false`	`DataRegion.split()`
the first row of a range is the minimum time, the last row the maximum	`TsFileProcessor` updates the tsfile resource boundaries
time partitions are crossed in one direction only	`InsertTabletNode.collectSplitRanges()`
the last row is the latest row	`InsertTabletNode.composeLastTimeValuePair()`

For the tablet [100, 15] of a device whose last flushed time is 39, 100 flips the sequence flag and 15 is classified as sequence as well, so it is written into a new sequence TsFile that overlaps the existing one.

Fix

Scope. Only the table-model INSERT INTO ... SELECT path. TableInsertTabletStatementGenerator#constructInsertTabletStatement() sorts the tablet before the statement leaves the INTO path, so it is ordered by the time it reaches the coordinator. (Earlier revisions of this PR sorted every tablet statement in the coordinator; this revision narrows the change to the INTO path.) The other ingresses — the tree model, REST, MQTT and the session API — build the statement from the request as received and are expected to submit ordered batches, as the storage layer documents; they are untouched here.

Sort. A stable sort by time: rows that share a timestamp keep their relative order, so the last one still wins when the storage layer resolves the duplicate. A batch that is already ordered only costs one pass over its timestamps and is not copied. It reuses the sorter the pipe already had — sortedness check, typed reorder of the value columns, bitmap handling, DATE handling — moved to org.apache.iotdb.db.utils.sorter, so there is a single implementation to keep correct and the query engine does not depend on the pipe package.

Attributes. Moving rows changes which row supplies the last non-null value of a device, and an attribute is device metadata rather than a timestamped field. The statement records the permutation the sort applied, and InsertTablet aggregates attributes in the submitted order, so the client's last row still wins.

Trade-off. A table-model tablet usually holds several devices, and sorting by time interleaves them: splitByDevice then yields one segment per contiguous run instead of one per device, and the write cost grows with the number of segments (10k-row tablets measured at ~1.2–2.4 ms grouped versus ~5.6–7.9 ms when most rows become their own segment). Sorting by device and then by time would keep the grouping, but it reorders rows across devices, which changes the device order that the storage layer, the session redirect metadata and the device attributes see; this change keeps to the time order only.

Known gaps

The TTL check still treats expired rows as a prefix, which a tablet holding several devices does not guarantee: a row marked out-of-ttl after the first alive row can be written. The scan applies the ttl lower bound, so such a row is filtered out at query time and never read back — the cost is storage, not correctness. A comment in InsertTabletNode#checkTTLInternal and TsFileProcessor#insertTablet records it for a follow-up.

Testing

InsertTabletStatementGeneratorTest, all against the table-model generator:

rows [100, 15] come out as [15, 100] with the values following their rows, and the permutation is recorded;
a null value moves together with its row, including the null bitmap;
an already sorted batch is left untouched and no permutation is recorded;
a full batch (rowCount == rowLimit, where the generator hands its own arrays to the statement) of mixed types is sorted;
a tag column moves together with its row, and the rows follow time order;
duplicate timestamps keep their relative order;
the generator is reusable after reset().

InsertTabletTest#testAttributeValueFollowsSubmittedRowOrderAfterSort checks that an attribute still comes from the last submitted row after the sort.

IoTDBInsertQueryIT#testInsertIntoQueryWithOutOfOrderTabletDoesNotOverlapSequenceFiles reproduces the situation end-to-end through SQL (INSERT INTO ... SELECT with an unsorted tablet, then SELECT) and asserts the rows come back in time order, in both scan directions.

The pipe sorter tests (PipeStatementEventSorterTest, PipeTabletEventSorterTest) are unchanged apart from the new package. All of the above were run locally on this branch.

Out of scope

Other ingresses. The tree model, REST, MQTT and the session API still rely on the documented contract that the client sorts the batch; a tablet submitted out of order through them can still produce overlapping sequence files.
Files that already exist on disk. This change prevents the overlapping state from being created; it does not change how such files are read. START REPAIR DATA moves an overlapping sequence file to the unsequence space.

🤖 Generated with Claude Code

codecov · 2026-10-28T05:28:48Z

Codecov Report

❌ Patch coverage is 88.23529% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.44%. Comparing base (65ef92f) to head (b4b58c0).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
.../evolvable/request/PipeTransferTabletRawReqV2.java	50.00%	2 Missing ⚠️
...ine/plan/statement/crud/InsertTabletStatement.java	60.00%	2 Missing ⚠️
...pache/iotdb/db/utils/sorter/InsertEventSorter.java	94.44%	2 Missing ⚠️
...ad/evolvable/batch/PipeTabletEventTsFileBatch.java	50.00%	1 Missing ⚠️
...db/pipe/sink/protocol/opcda/OpcDaServerHandle.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19035      +/-   ##
============================================
+ Coverage     48.29%   48.44%   +0.15%     
- Complexity      625      635      +10     
============================================
  Files          5572     5572              
  Lines        411719   412036     +317     
  Branches      54229    54314      +85     
============================================
+ Hits         198820   199631     +811     
+ Misses       212899   212405     -494

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JackieTien97

Thanks for the detailed root-cause analysis and the regression tests. I left a few inline comments, mostly about the cost the new sort adds to INSERT INTO ... SELECT and about how much of the problem the storage-layer guard actually covers. The numbers below were measured locally on the PR head (7f12bc27af; JDK 21, warmed-up JIT, 10,000-row batches).

shuwenwei · 2026-10-30T07:34:18Z

I would like to keep this PR scoped to the INSERT INTO ingress only.

The invariant violation originates because the internal INSERT INTO path bypasses the sorting performed by the Session API and constructs an InsertTabletStatement whose times are not sorted, although InsertTabletStatement documents that this invariant must hold. The minimal fix is therefore to restore that invariant inside InsertTabletStatementGenerator before the statement leaves the INSERT INTO path.

I do not think this PR should add defensive behavior to the general storage write path or to the query read path. Please remove the DataRegion.split() changes, the SeriesScanUtil changes, and the tests introduced solely for those defensive paths. Keep the tablet sorting in InsertTabletStatementGenerator, its unit tests, and an end-to-end INSERT INTO ... SELECT regression test proving that this path no longer produces incorrect results.

Once INSERT INTO generates a compliant sorted statement, it will no longer create the invalid sequence-file layout. Existing corrupted files and unsorted data from other ingress paths are separate concerns and should be handled in dedicated changes rather than partially masked in this PR.

Copilot

🟡 Changes recommended

The sorting changes leave critical watermark, RPC redirection, and pipe data-integrity defects unresolved, while the regression test lacks the multi-page setup needed to reliably expose the original failure.

4 open issues

What changed in this PR

This PR addresses #19020 by sorting tablet rows before storage routing, preventing out-of-order writes from creating overlapping sequence TsFiles. It also adjusts pipe time filtering for device-grouped tablets.

Changes:

Adds coordinator-side sorting and supplies column categories for table-model INSERT INTO ... SELECT.
Moves reusable tablet sorters into a shared utility package and updates their callers.
Adds sorting, pipe-filtering, and SQL regression coverage; documents the remaining TTL limitation.

File	Description
`iotdb-core/datanode/src/test/java/org/apache/iotdb/db/subscription/tagfilter/TabletTagFilterTest.java`	Uses the relocated table sorter.
`iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/execution/operator/process/InsertTabletStatementGeneratorTest.java`	Adds sorting, null, stability, and generator-reuse tests.
`iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/sink/PipeTabletEventSorterTest.java`	Updates sorter references.
`iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/sink/PipeStatementEventSorterTest.java`	Updates statement-sorter references.
`iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/event/PipeTabletInsertionEventTest.java`	Tests time filtering of device-grouped rows.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/TreeModelTabletEventSorter.java`	Relocates tree sorting and shares timestamp logic.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/TabletAdapter.java`	Relocates the tablet adapter.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/TableModelTabletEventSorter.java`	Adds stable device-and-time sorting without deduplication.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/InsertTabletStatementAdapter.java`	Relocates the statement adapter.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/InsertEventSorter.java`	Provides shared timestamp sorting and bitmap handling.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/sorter/InsertEventDataAdapter.java`	Relocates the shared adapter interface.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java`	Documents the TTL write-path limitation; no behavior change.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/crud/InsertTabletStatement.java`	Adds explicit device-ID cache invalidation.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/write/InsertTabletNode.java`	Documents non-prefix expired rows; no behavior change.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java`	Sorts tablet statements before planning and routing.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/process/TableInsertTabletStatementGenerator.java`	Attaches column categories to generated statements.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/opcda/OpcDaServerHandle.java`	Uses the relocated tree sorter.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/payload/evolvable/request/PipeTransferTabletRawReqV2.java`	Updates table and tree sorter references.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/payload/evolvable/request/PipeTransferTabletRawReq.java`	Updates tree sorter references.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/payload/evolvable/batch/PipeTabletEventTsFileBatch.java`	Uses shared sorters when buffering tablets.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tablet/PipeInsertNodeTabletInsertionEvent.java`	Avoids timestamp-endpoint filtering for multi-device tablets.
`iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tablet/parser/TabletInsertionEventParser.java`	Computes actual timestamp bounds before filtering.
`integration-test/src/test/java/org/apache/iotdb/relational/it/insertquery/IoTDBInsertQueryIT.java`	Adds SQL regression assertions for ordering and deduplication.

_{Review effort: Balanced}

Caideyipi · 2026-11-04T03:28:30Z

Reviewed commit 8f31c0911af96cb635edf231f2d45795b8898da0 against the PR merge base ca8074d15c824f2533d357778e40f773d02b5565 (23 changed files).

I do not recommend merging this revision yet. The following existing comments remain applicable after tracing the affected call paths:

Shared Pipe source arrays: table sorting replaces the statement's timestamps but mutates the source node's shared column/bitmap containers, which can leave timestamp/value pairs inconsistent for another consumer.
Session redirect ordering: redirect endpoints are generated in the server's reordered row order, while the client associates them with devices in the original request order.
Attribute update ordering: sorting before device-schema validation changes which input row supplies the last non-null attribute value for a device.
SQL regression page layout: the test setup still does not configure the multi-page layout that its reproduction requires.

No additional distinct actionable findings were confirmed in this review, so I have not duplicated the existing inline comments.

Validation: static review of the complete PR diff and relevant callers, schema-validation paths, sorters, and tests. No build, unit tests, or integration tests were run in this review.

ColinLeeo · 2026-11-04T05:12:54Z

The SonarCloud quality gate is failing on code this PR does not touch:

PipeResourceMetrics.java line 68 (java:S3077)
PipeTaskInfo.java line 1174 (java:S3078)

Both files are identical to master and neither appears in this PR's changed-file list. SonarCloud's pull-request analysis compares against the target branch's last analysis, which is from 2026-10-31 (26cbf0f6); the master commits merged into this branch since then (#19056, #19066, #19106) are therefore attributed to this PR as new code. Master's own quality gate currently fails as well.

Nothing in this PR can clear those two findings — they belong to a separate change (or disappear once master is analysed again). The rest of the gate (duplications, security, maintainability, coverage) is green, and the java:S2177 finding this PR did introduce is fixed in 8f31c0911a.

The sortedness check, the stable sort and the typed reorder of an insert tablet are needed by the write path too, so the sorter moves to org.apache.iotdb.db.utils.sorter and drops the Pipe prefix. It also used to reach into the pipe package for compactBitMaps; it now calls org.apache.iotdb.db.utils.BitMapUtils directly, so the query engine can use it without depending on pipe. The time-only sort moves from TableModelTabletEventSorter to its base class, because a statement sorter of either model can use it, and the base class now exposes the row permutation a sort applied, so a caller can keep the order the rows were submitted in. No behavior change otherwise: the pipe classes and tests only follow the new package.

`INSERT INTO ... SELECT` copies the rows of the child operator verbatim into the insert tablet, while the storage layer documents that it assumes the batch has been sorted by the client (InsertTabletNode#checkTTLInternal). The rows of a SELECT are not ordered: a join, an ORDER BY DESC or a scan over several devices can emit them in any order, so a tablet whose rows go backwards in time could write a row older than the device's last flush time into the sequence space. The resulting sequence TsFile overlapped the existing one, and queries returned duplicated rows and broke the time order, silently. Sort the tablet by time in TableInsertTabletStatementGenerator, i.e. only on the table-model INSERT INTO path, before the statement is submitted. The sort is stable, so rows that share a timestamp keep their relative order and the last one still wins, and it reuses the sorter that the pipe already had instead of a second implementation. A batch that is already ordered only costs one pass over its timestamps. Sorting moves rows, but an attribute is device metadata: it is the last non-null value of the rows as the SELECT emitted them. The statement records the permutation the sort applied and InsertTablet uses it, so the attribute still comes from the last submitted row. Other ingresses (the tree model, REST, MQTT, the session API) are out of scope: they are expected to submit ordered batches, as the storage layer documents.

Reproduce the overlap end to end through SQL: build a device whose sequence file is closed, insert the rows [100, 15] through INSERT INTO ... SELECT, and read them back. Without the sort the older row is written into a new sequence TsFile that overlaps the previous one, and the query returns a duplicated, unordered result; the test asserts the exact row sequence in both scan directions.

ColinLeeo · 2026-11-04T07:57:40Z

Reworked the scope after the review discussion — the description is updated.

The sort now lives in TableInsertTabletStatementGenerator#constructInsertTabletStatement() and covers only the table-model INSERT INTO ... SELECT path; it orders the rows by time before the statement leaves the INTO path.
The coordinator-wide reordering, the device-grouping sort and the pipe changes it required are gone. Those touched row order in places other ingresses depend on (redirect endpoints, attributes, the pipe time filter), which is more than this bug needs.
Device attributes keep the submitted order through the permutation the sort records, so "last non-null value of the client row wins" is unchanged.
The diff is now 21 files, mostly the sorter move plus the generator change and its tests.

The tree-model INTO path, REST, MQTT and the session API are explicitly out of scope: they still rely on the documented contract that the client submits an ordered batch. If we want to cover them later, the same sort can be invoked at their entry points.

The sort computes the row permutation anyway, so the timestamps can be rearranged with it in O(N) instead of being sorted a second time in O(N log N).

sonarqubecloud · 2026-11-05T08:36:36Z

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

ColinLeeo force-pushed the fix/tablet-out-of-order-seq-overlap branch from f56f2f7 to 851e1c7 Compare October 28, 2026 03:22

ColinLeeo force-pushed the fix/tablet-out-of-order-seq-overlap branch from 35286ac to 077029a Compare October 29, 2026 03:02

ColinLeeo changed the title ~~Fix sequence space overlap caused by out-of-order tablets~~ Fix duplicated and out-of-order rows when sequence TsFiles overlap Oct 29, 2026

ColinLeeo changed the title ~~Fix duplicated and out-of-order rows when sequence TsFiles overlap~~ Fix sequence space overlap caused by out-of-order tablets Oct 29, 2026

JackieTien97 reviewed Oct 29, 2026

View reviewed changes

ColinLeeo requested review from jt2594838 and shuwenwei October 29, 2026 08:21

ColinLeeo force-pushed the fix/tablet-out-of-order-seq-overlap branch 8 times, most recently from 80cd9d2 to 00022a2 Compare November 3, 2026 09:14

ColinLeeo requested a balanced review from Copilot November 4, 2026 01:43

Copilot started reviewing on behalf of ColinLeeo November 4, 2026 01:44

View session

ColinLeeo requested a review from Caideyipi November 4, 2026 01:45

Copilot AI reviewed Nov 4, 2026

View reviewed changes

ColinLeeo added 3 commits November 4, 2026 15:56

ColinLeeo force-pushed the fix/tablet-out-of-order-seq-overlap branch from 1ceb434 to 0c03539 Compare November 4, 2026 07:57

jt2594838 reviewed Nov 5, 2026

View reviewed changes

Rearrange the sorted timestamps with the row index

b4b58c0

The sort computes the row permutation anyway, so the timestamps can be rearranged with it in O(N) instead of being sorted a second time in O(N log N).

Conversation

ColinLeeo commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Fix

Known gaps

Testing

Out of scope

Uh oh!

codecov Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JackieTien97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shuwenwei commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Caideyipi commented Nov 4, 2026

Uh oh!

ColinLeeo commented Nov 4, 2026

Uh oh!

ColinLeeo commented Nov 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Nov 5, 2026

Quality Gate failed

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ColinLeeo commented Oct 28, 2026 •

edited

Loading

codecov Bot commented Oct 28, 2026 •

edited

Loading