Skip to content

Fix query partition memory retention and failure diagnostics - #19278

Merged
JackieTien97 merged 2 commits into
masterfrom
ty/cp-19269-master
Nov 14, 2026
Merged

JackieTien97 merged 2 commits into
masterfrom
ty/cp-19269-master

Conversation

@JackieTien97

@JackieTien97 JackieTien97 commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

Very large time-partition requests can keep query analysis and its memory reservations alive while RPC serialization repeatedly grows buffers. This change bounds serialization failures, checks query deadlines during partition enumeration, rejects overflowing memory reservations atomically, and records SQL/query-ID context for escaping failures without duplicate exception stacks.

Port the query-analysis and RPC fixes from #19269 (dev/1.3, through c7003fff631e252ae4f0796ae258124caffcf2a5) to master.

Example

For an uncompressed transport with a 64-byte frame limit and a peer at /127.0.0.1:6973, write a 64-byte prefix and then another 16 bytes. Previously, the second write buffered an oversized payload and the error appeared only at flush(). It now fails during the second write, before buffer growth, closes the transport, and preserves the destination in the diagnostic:

Serialized frame size is at least 80 bytes, exceeding the configured Thrift max frame size of 64 bytes to /127.0.0.1:6973.

Adaptations for master

  • Preserve RPC buffer memory accounting, temporary large-frame storage, and Thrift message-size tracking. Capture the remote endpoint before closing a socket, for both configured-frame and Java-array limits.
  • Apply non-retryable corrupted-frame handling in AbstractConfigNodeClient, where master implements ConfigNode RPC retries. Transient failures continue to retry.
  • Retain master's QUERY_EXECUTION_MEMORY_NOT_ENOUGH status (719) and adapt tests to the current exception, session, schema-fetcher, and transport-close APIs.
  • Provide matching English and Chinese diagnostics, including complete messages with and without an available destination.
  • Omit the dev/1.3-only coverage JDK workaround: master's baseline codecov job already succeeds without it.

Validation

Latest validation on JDK 21 with the Java 17 target:

  • 124 RPC tests across eight classes passed in a clean reactor build, including compressed/read transports, memory accounting, buffer reuse, and three new review regressions.
  • Both new endpoint regressions use real loopback TCP sockets and fail before the fix. They verify endpoint preservation, transport closure, and rejection before sending any bytes.
  • Removing the reuse-threshold guard makes the new crossing-within-capacity test fail; restoring it passes. The test verifies the 8 → 16 → 24 → 36-byte growth sequence, the switch to temporary storage, memory release, and payload round-trip.
  • Spotless, Checkstyle, English/Chinese key and placeholder parity, and both 50-module reactor compilation commands passed.
mvn clean test -pl iotdb-client/service-rpc -am \
  -Dtest=AutoResizingBufferTest,TElasticFramedTransportTest,TCompressedElasticFramedTransportTest,TElasticFramedTransportReadTest,DeepCopyRpcTransportFactoryTest,RpcUtilsTest,UrlUtilsTest,TSStatusCodeTest \
  -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
mvn test-compile -DskipTests
mvn test-compile -DskipTests -P with-zh-locale
  • Self-reviewed.
  • Ported regression tests and added coverage for master-specific behavior.

Port #19269 from dev/1.3 to master.

Preserve master RPC memory accounting, temporary large-frame buffers, and
message-size tracking; apply non-retryable frame handling in the shared
ConfigNode client. Adapt regression tests to current APIs and error codes,
and provide both English and Chinese diagnostics.

(cherry picked from commit 33277a6)
(cherry picked from commit 6a828ef)
(cherry picked from commit 27c44f5)
(cherry picked from commit 856d4a0)
(cherry picked from commit 827763a)
(cherry picked from commit 86b83e0)
(cherry picked from commit c7003ff)
@codecov

codecov Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.09910% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 49.18%. Comparing base (e78f736) to head (6185c34).

Files with missing lines Patch % Lines
...e/iotdb/db/queryengine/common/MPPQueryContext.java 93.75% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19278      +/-   ##
============================================
+ Coverage     49.16%   49.18%   +0.02%     
  Complexity      689      689              
============================================
  Files          5593     5593              
  Lines        414614   414715     +101     
  Branches      54755    54770      +15     
============================================
+ Hits         203841   203984     +143     
+ Misses       210773   210731      -42     

☔ 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 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed as a port of #19269 (dev/1.3 final c7003fff63). I built master base e78f736613 and this head and ran each as a 1C1D cluster (Windows, JDK 21, dn_thrift_max_frame_size=1048576). Both clusters had 64 devices in a tree database and 64 in a table. I also ran transport and Session probes against the same jars.

Equivalence. Every change from #19269 is here. The master adaptations hold up:

  • i18n: the placeholders in the new constants match their call sites in both en and zh.
  • Retry loop: the non-retryable CORRUPTED_DATA branch now lives in AbstractConfigNodeClient. The DataNode ConfigNodeClient is its only subclass, so the scope is unchanged.
  • Fast path: its two extra exits are both needed. A closed reusable buffer must re-reserve memory, and a write crossing maxReusableBufferSize must still go through the temporary buffer. Temporary-buffer growth is capped at the limit too. Removing the first exit fails testFramedFastPathReservesMemoryAfterReopening; nothing pins the second (inline nit).
  • Table model: covered. PushPredicateIntoTableScan calls the same getTimePartitionSlotList, and every executeFor* path goes through Coordinator#execution.

Measured

master base this head
oversized partition request, tree model 15,403 ms, "Fail to connect to any config node" (15 retries) 23–36 ms, frame-size message
same, table model 15,379 ms, same message 18–28 ms, frame-size message
small-field serialization, ns/slot 10.37–10.62 9.17–9.47
large binary serialization, ns/row 1.72–1.81 1.75–1.81

The serialization rows are a microbenchmark into a null sink, median of 3 alternating JVM runs.

Other results on this head:

  • time >= 0 and time <= 9223372036854775806 returns 719 with the time-partition memory diagnostics, in both models.
  • [QueryFailed] is logged at INFO without a stack in both models, and the 305 stack is logged once.
  • Capacity at rejection is exactly the limit (1.00×), including when the temporary buffer is in use (thresholds 1 MiB and 256 KiB).
  • A Session with thriftMaxReusableBufferSize(262144) inserted 50,000 random rows through the temporary buffer, and they read back with 0 mismatches.

CI

  • The SonarCloud gate is red only because of java:S9365 in InsertValues.java:95, a file this PR doesn't touch. Its PR analysis also attributes other recent master changes to this PR.
  • AINode (ubuntu-latest) was cancelled rather than failed, so it needs a re-run.

Three inline comments: a P3 on the new error text, plus two nits (test coverage and the workflow).

Comment thread .github/workflows/sonar-codecov.yml Resolved
Capture the destination before closing a socket when rejecting oversized
writes, and provide complete English and Chinese diagnostics.

Cover writes that cross the reuse threshold while fitting the current
array, and remove the coverage JDK workaround that master does not need.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed 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

@JackieTien97
JackieTien97 merged commit dea6e2b into master Nov 14, 2026
41 of 43 checks passed
@JackieTien97
JackieTien97 deleted the ty/cp-19269-master branch November 14, 2026 14:15
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.

1 participant