Skip to content

Fix receiver write admission stalls and RPC buffer allocation failures - #18918

Merged
jt2594838 merged 3 commits into
apache:masterfrom
Caideyipi:fix/pipe-receiver-memory-pressure
Oct 21, 2026
Merged

jt2594838 merged 3 commits into
apache:masterfrom
Caideyipi:fix/pipe-receiver-memory-pressure

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Large tablet transfers can encounter receiver stalls in two memory-control paths:

  • An insert holding the region write lock can wait for a different TsFileProcessor in the same region to flush, while the asynchronous flush submitter needs that lock. Mark the current memtable so the writer can finish and release the lock. Admission retains the hard memtable limit, normal selection prefers the largest memtables, and a first tablet whose memory is reserved before it contains data can still progress. Repeated reports count an already marked memtable without submitting another flush task; replacement memtables can be selected normally.
  • RPC buffer shrinking must use a complete frame's size. Incremental serialization only ensures capacity; ordinary and compressed framed transports evaluate shrinking after writing the complete frame. This prevents large-frame shrink/grow cycles while allowing genuinely small frames, including those at or below initial capacity, to release excess reservations after the existing count and cooldown. Optional 1.5x growth falls back to the capacity required by the frame when headroom does not fit. Failure preserves the existing frame, buffer state, and accounting.

Evidence and remaining validation

The raw REPORT.md receiver logs contain 6,833 / 4,276 / 11,478 distinct allocation failures. All are on read-buffer growth except 21 initial 1 KiB read-buffer allocations on node 46. They show shared RPC-budget exhaustion, not proof that the PR's write-buffer regression caused those failures. The write regression is independently reproducible. Controlled concurrent read-budget rejection/release/retry behavior is also reproducible using buffer implementations from before the current branch's last six commits.

The fallback can still close a small current TsFileProcessor through DirectFlushPolicy. Avoiding duplicate submissions does not eliminate that small-file risk. A same-workload old/new baseline covering flush counts, memtable/file-size distributions, write progress, and RPC memory pressure is still required. This change does not claim that all workload read-allocation failures or small-file amplification have been eliminated.

Validation

At ab56b84:

  • mvn -pl iotdb-client/service-rpc verify: 108 English tests passed; dependency analysis, forbidden-API checks, and RAT passed.
  • Chinese AutoResizingBufferTest: 19 tests passed.
  • SystemInfoTest and TsFileProcessorTest: 10 + 26 tests passed.
  • Spotless and Checkstyle passed for service-rpc and datanode.
  • Against the original PR, the strengthened suites reproduced 3 buffer assertion failures and 4 SystemInfo assertion failures; all pass with the fix.

Regression coverage includes repeated large frames beyond the shrink count, unfinished frames written in small fragments, frame integrity, small-frame shrink count/cooldown and minimum capacity, shared budgets between connections, required-capacity fallback, failed-write rollback, close/release accounting, duplicate flush scheduling, first-tablet progress, and hard-limit rejection.

This PR has:

  • been self-reviewed.
  • added comments explaining the intent of the changes.
  • added regression tests.

Comment on lines +140 to +151
if (chooseMemTablesToMarkFlush(tsFileProcessor)) {
boolean currentProcessorSelected = chooseMemTablesToMarkFlush(tsFileProcessor);
if (!currentProcessorSelected
&& tsFileProcessor != null
&& tsFileProcessor.getWorkMemTable() != null) {
// The writer holds its region's write lock. Waiting for another processor in that region
// to flush would prevent the flush submitter from acquiring the same lock. Let this writer
// finish and flush its own memtable, subject to the hard memory limit below.
tsFileProcessor.setWorkMemTableShouldFlush();
flushTaskSubmitThreadPool.submit(tsFileProcessor::submitAFlushTask);
currentProcessorSelected = true;
}
if (currentProcessorSelected) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may result in flushing many small files.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this is a valid risk and should be addressed before merging. The fallback can flush the current processor even when its memtable is much smaller than the processors selected by the normal policy. The workload report shows 172/280/458 reject-status events and zero rejected inserts/606 errors, but it does not include flush counts or memtable/file-size distribution, so it cannot rule out small-file amplification. I will treat this as unresolved and collect those metrics under the same workload before claiming the fallback has no such cost.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up: I reproduced a separate RPC regression in fa77c9350c2. The framed-transport flush reset passes the initial buffer size to resizeIfNecessary; with this PR's new shrink eligibility for requests at or below the initial size, repeated large frames shrink a 1,000-byte write buffer to 550 bytes after the cooldown/count, then require it to grow again. Under a shared memory cap, that churn can make another live buffer's allocation fail. An identical four-case harness passes against the pre-PR parent, fails three cases against fa77c9350c2, and passes all four with a local fix that shrinks from the completed frame size and only grows during incremental serialization. The local service-rpc suite passes (51 English tests; 18 Chinese tests), as do 25 focused storage tests.

This is related, but it does not explain all errors in REPORT.md: parsing the receiver logs shows 6,833 / 4,276 / 11,480 distinct allocation-failure events, all but 21 on node 46 are read-buffer frame growth failures. Node 46 also has 21 constructor failures for 1 KiB initial buffers, which indicates the shared 819 MiB RPC budget was exhausted. The PR's unnecessary write-buffer churn can add pressure, while the existing read failures still need concurrent-budget investigation. This thread remains open pending that work.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed ab56b84 with the RPC capacity fix and protection against repeated flush submission.

The fallback is retained: the inserting thread holds the region write lock, so waiting for another processor's flush can prevent that flush submitter from acquiring the same lock. Both the normal selection and fallback now reuse an already marked memtable's pending flush instead of enqueueing another task on every report. A replacement memtable can still be scheduled. Tests cover small/first-tablet memtables, repeated reports, normal largest-first selection, rejection recovery, and rejection at and above the hard limit.

This does not establish that the small-file risk is eliminated. DirectFlushPolicy still closes the selected TsFileProcessor, and legitimately selecting a small current memtable can still produce a small file. The existing close-future guards make repeated close requests idempotent, but do not remove that policy tradeoff. A matched old/new workload comparison of flush counts, memtable sizes, file sizes, and write progress is still required; I am leaving this discussion unresolved.

Separately, incremental RPC writes now only ensure capacity; both framed transports make their shrink decision using the completed frame size. The optional-growth fallback to the required capacity and failure accounting remain covered. Against the original PR, the strengthened buffer suite reproduced 3 failing tests and SystemInfoTest reproduced 4; after the fix, service-rpc verify passed all 108 English tests, the Chinese buffer suite passed 19, and SystemInfoTest plus TsFileProcessorTest passed 36. Spotless, Checkstyle, and the verify lifecycle checks passed.

I also rechecked REPORT.md's raw receiver logs: 6,833 / 4,276 / 11,478 distinct failures; all are read-buffer growth failures except 21 initial 1 KiB read-buffer allocation failures on node 46. Startup logs show a shared RPC budget of 858,993,459 bytes. A controlled read-budget rejection/release/retry test passes with the buffer and memory-account implementations from before the current branch's last six commits as well as the original PR. This supports budget exhaustion as the immediate mechanism, but neither that test nor the logs establish which change caused the workload's pressure. The independently reproduced write shrink/grow regression is fixed; eliminating all workload read-allocation failures remains unverified.

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Correction after parsing the raw receiver error logs: REPORT.md counts both the outer TTransportException and its Caused by line, so distinct failures are 6,833 (node 44), 4,276 (45), and 11,478 (46). The stacks identify 6,833/4,276/11,457 as read-buffer growth failures in AutoScalingBufferReadTransport.fill -> TElasticFramedTransport.readFrame; node 46 has 21 additional failures allocating the initial 1 KiB buffer. I found no write-buffer allocation failures. The failed read-frame allocations have median sizes of about 13.2/14.0/13.8 MB, with a maximum near 31.9 MB. This shows shared RPC-budget exhaustion during concurrent reads, but does not by itself establish that this PR caused those read failures. The write-buffer shrink/grow regression remains independently reproducible and may add contention on that shared budget. For the report window 11:44:39-12:29:06, reject-status events are 170/0/210 for nodes 44/45/46. The totals 172/280/458 extend beyond that window; node 45 first reports one at 12:29:11. Compression logs within the same window show 3/1/13 files below 1 MiB and 37/3/80 below 10 MiB. Without a same-workload old-build baseline, these counts do not quantify PR-induced small-file amplification. I recommend holding approval until the write-buffer regression is fixed and the fallback flush is compared with aligned flush/file-size metrics. This remains unresolved.

@jt2594838 jt2594838 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert the flush-triggering condition.

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Applied in commit 346d355.

I reverted the reject-path fallback that marked and submitted the current processor when the normal largest-first selection did not choose it. The duplicate flush-submission guard remains, as do the RPC frame-buffer fixes. SystemInfoTest now verifies that the fallback cannot bypass selection, while still covering replacement memtables and the hard memory limit when the current processor is selected.

Validation: SystemInfoTest 10/10 and AutoResizingBufferTest 19/19 passed. I am leaving this discussion unresolved.

@jt2594838
jt2594838 merged commit 83d4197 into apache:master Oct 21, 2026
39 of 43 checks passed
@jt2594838
jt2594838 deleted the fix/pipe-receiver-memory-pressure branch October 21, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants