[chrisdutz]

Description

MemTableFlushTask.syncFlushMemTable() reserves temporary memory for a flush and takes it out of
the MemTable budget (SystemInfo.reserveTemporaryMemoryForFlushing, since #18848; before that
applyTemporaryMemoryForFlushing with the same estimate):

memSize / seriesNumber * io_task_queue_size_for_flushing

The bounded IO queue holds up to io_task_queue_size_for_flushing (default 10) encoded series of
average size. With fewer series than queue slots the formula estimates more than the memtable
itself: a memtable with 3 series reserves 10/3 = 3.3 times its size. With a small MemTable budget
a single flush can then reserve more than the whole budget, so FLUSH_THRESHOLD and
REJECT_THRESHOLD become negative and the node rejects writes until the flush is done.

Measured with the same code in a downstream fork (standalone DataNode, 147.9 MB MemTable budget),
writing to one device with 3 series (flushes of about 52 MB):

	largest reservation	lowest reject threshold
before	174.5 MB	-21.3 MB (rejecting)
after	52.4 MB	76.4 MB
before, 100 devices x 3 series (not affected)	1.7 MB	116.9 MB

The queue can never hold more than the memtable, so the estimate is now capped at memSize
when there are no more series than queue slots; with more series it is unchanged.

The estimate is moved into a small static method estimateTemporaryMemory(...) so that it can
be unit tested (FlushMemoryEstimateTest).

Tests

• New FlushMemoryEstimateTest (fails before the change: 173.3 MB estimated for a 52 MB memtable
  with 3 series).
• MemTableFlushTaskTest, TsFileProcessorTest, SystemInfoTest, DataRegionTest: 106 tests
  pass; spotless:check passes.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.89%. Comparing base (4b80fa5) to head (de0924b).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19108      +/-   ##
============================================
+ Coverage     48.43%   48.89%   +0.46%     
- Complexity      635      689      +54     
============================================
  Files          5572     5588      +16     
  Lines        412012   413448    +1436     
  Branches      54309    54538     +229     
============================================
+ Hits         199556   202171    +2615     
+ Misses       212456   211277    -1179     

☔ 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.

[chrisdutz]

The failing SonarCloud quality gate (new_reliability_rating = 3) is not caused by this change: it comes from two bugs in recently merged pipe code and currently fails for every pull request. #19109 fixes them (its own SonarCloud gate passes); once it is merged, the gate here should pass on the next analysis.

[JackieTien97]

@jt2594838 @HTHou plz take a look

[HTHou]

Could we leave some headroom for the encoded chunk buffers here? The I/O queue holds chunk writers, and a single series can enqueue multiple chunks. The writers' backing arrays can also grow larger than the raw data counted by memSize().

I reproduced this by pausing I/O until the queue filled, using one device with nine DOUBLE series (PLAIN, UNCOMPRESSED, 60,000 points per series). The queued writers' backing arrays alone occupied 9,406,224 bytes on both revisions. The base reserved 9,600,000 bytes, while this PR reserved 8,640,000 bytes. That leaves 766,224 bytes unaccounted for when computing the write rejection threshold.

Can we keep the reduction in over-reservation while accounting for those buffers, and add a regression test that exercises the reservation with a stalled I/O consumer?

[chrisdutz]

Thanks, good catch. I could reproduce it: with I/O stalled until the queue is full, one device with nine DOUBLE series (PLAIN, UNCOMPRESSED, 60,000 points each) gives 9,441,072 bytes of queued chunk buffers (ChunkWriterImpl.pageBuffer) against a memSize() of 8,640,000. I only got numbers close to yours with the time column PLAIN as well; with the default TS_2DIFF time encoder the buffers stay at about half of the reservation.

The data written into the buffers matches memSize() almost exactly (8,649,045 bytes). The difference is the spare capacity the buffers keep after growing by doubling. The old estimate doesn't cover it either. It only came out ahead in this case because ten queue slots for nine series add about 11%. With more series than slots, both versions reserve the same amount and both come up short:

Series × points	Reservation: master / this PR so far	Queued buffers	Buffers / reservation
9 × 60,000	9,600,000 / 8,640,000	9,441,072	0.98 / 1.09
30 × 60,000	9,600,000 (both)	10,490,080	1.09
300 × 6,000	960,000 (both)	1,311,540	1.37

Since the buffers' capacity can approach twice their content, the estimate now doubles the queued share. The cap on the raw share stays, so the reservation is at most twice the memtable, instead of up to ten times (the queue size divided by the number of series) before this PR.

MemTableFlushTaskTest.testReservationCoversQueuedChunkBuffersWithStalledIo is the regression test you asked for. It stalls the I/O consumer, waits until the encoder has filled the queue, and checks that the reservation actually taken from SystemInfo covers the capacity of the queued chunk buffers, for 9 × 60,000, 1 × 60,000 and 300 × 6,000 (time and values PLAIN). It fails without the factor (9,441,072 B of buffers against 8,640,000 B reserved) and passes with it.

One trade-off: the flushing memtable keeps counting towards the reject threshold until the flush ends, and the reservation lowers that threshold. In small memory configurations with few series per memtable, a 2× reservation can therefore put writes into reject status for the duration of a flush. If you think that matters, I could make the factor configurable (e.g. temporary_memory_factor_for_flushing, default 2.0), so such setups can lower it. I left it out to keep this PR small.

[JackieTien97]

Found one remaining under-reservation case in the updated flush memory estimate; reproduction details are inline.

[JackieTien97]

[P2] Account for encoding expansion before applying the buffer growth factor

The 2× growth bound applies to the encoded byte count, but memSize() counts raw values. For example, PLAIN-encoded INT32 values can occupy 5 bytes each while memSize() counts only 4, so doubling the raw share can still under-reserve the queued buffers.

I reproduced this on d458e7be55 using the full MemTableFlushTask pipeline with I/O stalled at startChunkGroup: one device, one non-aligned INT32 series, 41,000 rows, timestamps i, values Integer.MAX_VALUE - i, PLAIN time/value encoders, and UNCOMPRESSED data. With the default queue/chunk settings, the queue contains one chunk writer:

Measurement	Bytes
memSize()	492,000
Retained buffers, measured with this PR's retainedArrayBytes helper	1,048,912
Actual reservation with this PR	984,000
Actual reservation with the base flush implementation	4,920,000

The same buffer-coverage assertion fails with the PR implementation and passes with the base implementation. This leaves at least 64,912 bytes unaccounted for when reducing the MemTable budget and computing the write-rejection threshold. The existing 18 targeted tests pass, but the new stalled-I/O cases only exercise DOUBLE values.

Could we account for encoding expansion and page overhead before applying the capacity-growth allowance, and add this INT32 case to the stalled-I/O regression test?

[chrisdutz]

Thanks, confirmed: with your INT32 case I get the same 1,048,912 bytes of queued buffers against a reservation of 984,000.

To see how far this goes, I measured the encoded size against memSize() for every data type and encoding with random full-range values (PLAIN time column). Variable-length integers are the worst case: about 1.08 for PLAIN and ZIGZAG INT32 and 1.09 for ZIGZAG INT64, with a bound of 1.25 for the values themselves (5 bytes for an int, 10 for a long). All other encodings stay within about 6% (RLBE is the highest), and TEXT and BOOLEAN with RLE stay below memSize(). Page headers are included in these numbers.

Since the buffers' capacity can reach twice their content, the estimate now multiplies the queued share by 2.5 (2 × 1.25) instead of 2, still capped at the memtable size (de0924b). Your INT32 case (one series, 41,000 rows, Integer.MAX_VALUE - i, PLAIN, uncompressed) is now part of testReservationCoversQueuedChunkBuffersWithStalledIo; it fails with the factor 2 and passes with 2.5.

I also considered a per-series bound from the encoders' maximum item sizes, which would be tighter, but it means walking the memtable's schemas for the estimate. I kept the constant for now; happy to switch if you prefer that.

[sonarqubecloud]

Quality Gate failed

Failed conditions
B 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