Repository navigation
Fix query partition memory retention and failure diagnostics - #19278
Conversation
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 Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
JackieTien97
left a comment
There was a problem hiding this comment.
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_DATAbranch now lives inAbstractConfigNodeClient. The DataNodeConfigNodeClientis 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
maxReusableBufferSizemust still go through the temporary buffer. Temporary-buffer growth is capped at the limit too. Removing the first exit failstestFramedFastPathReservesMemoryAfterReopening; nothing pins the second (inline nit). - Table model: covered.
PushPredicateIntoTableScancalls the samegetTimePartitionSlotList, and everyexecuteFor*path goes throughCoordinator#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 <= 9223372036854775806returns 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:S9365inInsertValues.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).
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.
|




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, throughc7003fff631e252ae4f0796ae258124caffcf2a5) tomaster.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 atflush(). It now fails during the second write, before buffer growth, closes the transport, and preserves the destination in the diagnostic:Adaptations for master
AbstractConfigNodeClient, where master implements ConfigNode RPC retries. Transient failures continue to retry.QUERY_EXECUTION_MEMORY_NOT_ENOUGHstatus (719) and adapt tests to the current exception, session, schema-fetcher, and transport-close APIs.Validation
Latest validation on JDK 21 with the Java 17 target:
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