Skip to content

Refactor write memory accounting into named memory blocks - #18848

Open
hongzhi-gao wants to merge 4 commits into
apache:masterfrom
hongzhi-gao:refactor/write-memory-blocks
Open

hongzhi-gao wants to merge 4 commits into
apache:masterfrom
hongzhi-gao:refactor/write-memory-blocks

Conversation

@hongzhi-gao

@hongzhi-gao hongzhi-gao commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

Write-memory accounting is spread across MemTables, TsFileProcessors and DataRegions, requiring callers to update several counters together. This change introduces named, categorized owner blocks that propagate usage to their accounting scope and tie rollback/release to the owner's lifecycle.

Accounting and lifecycle

  • Add WriteMemoryBlock and WriteMemoryManager for MemTable arrays, Binary values, recovered totals, TsFile metadata and flush temporary reservations. Pending reservations count toward admission but become used MemTable memory only after admission succeeds. WAL failures roll back the admitted deltas.
  • Replace positional write-memory deltas with named fields across row, mixed-row and split-tablet paths. MemTable release closes its blocks and updates the region total once; query-held TVLists retain their existing resource ownership.
  • Preserve the WAL snapshot's aggregate memory field. Recovery places that aggregate in a separate category rather than guessing an array/Binary breakdown.
  • Scope flush reservations to actual worker lifetime: propagate worker failures, cancel and await both workers, clear temporary references, then release the reservation. Budget reload retains outstanding reservations.
  • Store each WAL queue entry's original charge and refund it on dequeue or interrupted enqueue.

Array-pool accounting and hot paths

Count idle arrays by actual shallow size under the existing per-type queue locks. A new ReadOnlyMemoryBlock exposes an exact snapshot to the existing memory-manager tree; readers acquire queue locks in a fixed order. This avoids an additional global atomic update on each array borrow/return. WriteMemoryBlock uses a monitor-guarded long; region totals remain atomic. Zero-delta writes skip reservation creation while retaining pressure checks.

SQL, configured memory limits, file/WAL formats and the existing admission policy are unchanged. Accounting follows write ownership, not total JVM heap liveness. New exception messages are provided in both English and Chinese.

Verification

Added 4 lifecycle integration tests, all passed in a real 1 ConfigNode + 1 DataNode environment using the repository’s fixtures, run through JUnitCore.

The tests cover:

  • Array allocation and reuse: write across the 63/64/65-row boundaries, introduce sparse and new aligned columns, then repeat flushes and overwrite previously flushed data through unsequence writes.
  • Rejection and retry: submit oversized WAL entries through row and Tablet writes, check that rejected writes leave no residual memory charge, and verify subsequent writes succeed.
  • Restart and recovery: force a DataNode restart, check recovered data and write-memory accounting, then verify subsequent writes and flushes.
  • Table-model split writes: write Tablets spanning multiple devices and time partitions, checking data and accounting across the resulting splits.

Assertions check both query results and live database write-memory metrics, including memory charged by writes and released after flush or rejection.

Performance observations

Compared with eb0e3f1f2080e34bf5593bfd1cf63b62ecdba2e0, using the same workload/configuration and three interleaved runs per version:

Workload (4 writers) Median throughput P50 RPC latency P99 RPC latency
Non-aligned Tablet, 1024 rows -0.24% +3.88% +0.10%
Aligned single row +0.50% -0.73% +3.19%
Aligned Tablet, 1024 rows +4.36% -4.60% -3.58%

Environment: shared Xeon Gold 6130 KVM host, 32 vCPUs, JDK 21, 1C1D/SimpleConsensus, ASYNC WAL, memory control enabled, 2 GiB DataNode heap, array size 64, MemTable threshold 32 MiB, compaction and timed flush disabled. Fixed per-writer warmup/measurement calls were 50k/100k for non-aligned Tablet, 125k/250k for aligned rows, and 20k/40k for aligned Tablet. Each run checked acknowledged row counts through queries after a final FLUSH.

These are controlled class-overlay comparisons, not two independent clean distribution builds. The throughput window excludes the final FLUSH; it does not measure synchronous durability throughput. Shared-host variability and existing QueryId waits affect single-row results. The repeated throughput regression observed during development did not recur in the final comparisons, but the latency increases above remain review considerations; these measurements do not establish a universal speedup.


This PR has:

  • been self-reviewed, including concurrent reads, writes and read/write interaction.
  • added comments explaining accounting ownership, reservation ordering and snapshot lock ordering.
  • added unit/component tests for lifecycle and failure paths.
  • added integration tests and run them against a real 1C1D environment.
Key changed/added classes

WriteMemoryBlock, WriteMemoryManager, AbstractMemTable, TsFileProcessor, DataRegionInfo, TsFileProcessorInfo, MemTableFlushTask, SystemInfo, MemoryControlledWALEntryQueue, PrimitiveArrayManager, ReadOnlyMemoryBlock, MemoryManager.

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed commit ef0d5bfa35c3 against the PR's merge base eb0e3f1f2080. I did not find an actionable regression introduced by this change.

The review covered row, mixed-row and split-tablet memory admission/WAL rollback; MemTable and metadata release; WAL snapshot format compatibility; idle-array accounting; and flush-worker failure/interruption cleanup.

Validation

89 focused tests passed:

  • 59 DataNode tests across WriteMemoryBlockTest, MemTableMemoryBlockTest, WriteMemoryLifecycleTest, PrimitiveArrayManagerTest, MemTableFlushTaskTest, and TsFileProcessorTest.
  • 3 tests in ReadOnlyMemoryBlockTest.
  • 27 tests in AlignedTVListTest.

git diff --check also passed.

Full-reactor test compilation could not complete because the local JVM failed to reserve native memory. The integration tests in this PR were not rerun as part of this review.

Comment on lines +457 to +469
private boolean awaitExit() {
boolean interrupted = false;
while (true) {
try {
finished.get();
return interrupted;
} catch (InterruptedException e) {
interrupted = true;
} catch (ExecutionException e) {
throw new AssertionError(e);
}
}
}

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.

Why does the interruption not exit?
And why is the ExecutionException wrapped with AssertionError?

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.

Replaced the exit wait with a CountDownLatch, so worker failures are propagated through subTaskFailure to syncFlushMemTable rather than being represented as an AssertionError in cleanup. The caller still has to wait after interruption: cancellation only requests worker interruption, and the worker may still be using the writer/buffers. Returning here would release its flush-memory reservation before those resources stop being used. Cleanup waits for both workers to exit, then restores the caller's interrupt flag and propagates the original failure/interruption. The regression now also interrupts the caller again during cancellation and checks that the reservation remains live until the I/O worker exits.

Implemented in f1945e5201.

Validation: 87 focused tests passed; full-reactor test compilation passed for both the default and Chinese locales; all 4 IoTDBWriteMemoryLifecycleIT cases passed in 1 ConfigNode + 1 DataNode, including forced-restart WAL recovery. Independent Codex review found no actionable issues.

Comment on lines +87 to 89

/** Adds array/TVList structural memory; Binary payload has a separate block. */
void addTVListRamCost(long cost);

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.

Add a structureMemoryBlock?

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.

Added getStructureMemoryBlock() and the MEMTABLE_STRUCTURE category. Aligned TVList container estimates are now accounted separately from arrays/bitmaps and Binary payloads, using the existing before/after reconciliation for working and sorted TVLists. This keeps the aggregate estimate unchanged rather than adding a second charge. The block participates in attach, clear and release; WAL snapshots retain the existing aggregate field and recover it into the recovered-total block. Extended coverage checks category totals, WAL-failure rollback, clear/release, and both old/current snapshot readers.

Implemented in f1945e5201.

Comment on lines +308 to +323
private static long arraySize(Object array) {
// Use typed array lengths on this hot path instead of reflective shallowSizeOf(Object).
if (array instanceof long[]) {
return RamUsageEstimator.sizeOf((long[]) array);
} else if (array instanceof int[]) {
return RamUsageEstimator.sizeOf((int[]) array);
} else if (array instanceof boolean[]) {
return RamUsageEstimator.sizeOf((boolean[]) array);
} else if (array instanceof float[]) {
return RamUsageEstimator.sizeOf((float[]) array);
} else if (array instanceof double[]) {
return RamUsageEstimator.sizeOf((double[]) array);
}
// Binary[] entries have been cleared; only the shallow reference array belongs to the pool.
return RamUsageEstimator.shallowSizeOf((Binary[]) array);
}

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.

How about caching the size calculators in an array consistent with POOLED_ARRAYS to reduce unnecessary type judgement.

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.

Added cached typed size calculators indexed by TSDataType.serialize(), matching POOLED_ARRAYS. Borrow/return now use the calculator for the selected pool instead of repeating the instanceof chain to calculate its size. The calculators still use the actual array length, and Binary[] accounting remains shallow after clearing its elements. Added coverage for the supported types and their primitive/Binary pool aliases; existing variable-length and concurrent pool-accounting regressions also pass.

Implemented in f1945e5201.

Preserve explicit data-model handling and flush worker lifetime accounting. Adapt PR-only tests to the updated flush and memchunk APIs.
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.

3 participants