Skip to content

Adapt file-scoped TsFile contexts and resource lifecycles - #18999

Merged
HTHou merged 7 commits into
apache:masterfrom
HTHou:codex/tsfile-context-lifecycle
Oct 26, 2026
Merged

HTHou merged 7 commits into
apache:masterfrom
HTHou:codex/tsfile-context-lifecycle

Conversation

@HTHou

@HTHou HTHou commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Description

Upgrade TsFile to 2.5.1-261023-SNAPSHOT and adapt the storage/query integration to its file-scoped encryption contexts. This PR adds reusable integration points only; it does not add a crypto implementation, wallet, key manager, WAL encryption, or a new user configuration option.

Reader APIs and Resource Ownership

  • Use the reader-reported data start offset and pass chunk ordinals/page indexes to page deserialization in query, Pipe, Load, repair, and split paths.
  • Retain file-scoped contexts for lazy pages until their last page is consumed, or until query/iterator cleanup. Only page-AEAD chunks enter the query ownership registry; ordinary chunks are not strongly tracked.
  • Release cached chunks on eviction, retain returned chunk contexts independently, and handle eviction racing with a cache hit.
  • Close partially constructed aligned readers and Pipe iterators on errors and early termination.

Per-File Writing and Rewrite Policy

  • Add the optional FileEncryptionParameterProvider SPI. The default uses the existing legacy parameters unchanged. No production provider is bundled. Ambiguous providers and a custom provider returning null fail explicitly.
  • Create fresh file parameters through shared writer factories and propagate the writer's context to memtable/chunk writers, including target-file switches during cross compaction.
  • Support per-target-file contexts in ReadPoint compaction and a rewrite-policy hook for Load through the existing tablet write pipeline. Source deletion remains the Load scheduler's responsibility.
  • Preserve subclass measurement initialization when using the new chunk-writer factory overload, including the sorting buffers used by repair compaction.
  • Fast/ReadChunk compaction adaptation and automatic redirection are intentionally out of scope. Extensions using file-scoped compaction contexts must select ReadPoint; this PR does not claim encrypted Fast/ReadChunk support.
  • Preserve existing plain/legacy behavior when no custom policy is installed. The test provider is framing-only and lives entirely in test sources.

Verification

  • Targeted DataNode tests: 136 tests, 0 failures, 0 errors, 3 skipped. Covers file-context lifecycle, aligned/non-aligned lazy readers, cache eviction, plain files, memtables/flush, Load/splitting, Pipe, ReadPoint compaction, and the complete RepairUnsortedFileCompactionTest / RepairUnsortedFileSchedulerTest classes. Existing plain-file Fast/ReadChunk cases are also run as regressions, not as encrypted-format support tests.
  • Full-reactor English and Chinese test-source compilation, including the integration-test module.
  • Spotless and git diff --check.

No running-cluster integration test was executed for this PR.


This PR has:

  • been self-reviewed, including cache eviction/read ownership.
  • added Javadocs for the new provider contract and resource ownership.
  • updated the TsFile dependency version.
  • added unit tests and exercised existing regression tests.
  • been tested in a test IoTDB cluster.
Key Changed/Added Classes

FileEncryptionParameterProvider, EncryptDBUtils, QueryChunkReaders, ChunkCache, DiskChunkLoader, DiskAlignedChunkLoader, CompactionTsFileWriter, AbstractCompactionWriter, TabletRewriteLoadStrategy, and TsFile-based Pipe/Load readers.

@shuwenwei

Copy link
Copy Markdown
Member

DiskAlignedChunkLoader may double-close chunks on failure

At DiskAlignedChunkLoader.getChunkReader() (around L127-L147), the catch block closes timeChunk and every chunk in valueChunkList. However, in TsFile 2.5.1:

  • AlignedChunkReader / TableChunkReader already closes its chunks if initAllPageReaders() fails.
  • QueryChunkReaders.retain() also closes the reader before throwing when the query resource registry is already closed.

Therefore failures in either new AlignedChunkReader(...) / new TableChunkReader(...) or QueryChunkReaders.retain(...) can execute Chunk.close() twice. Chunk.close() releases a page-AEAD EncryptParameter reference and is not idempotent, so this can over-release the file-level context and destroy the provider context too early.

Suggested fix: track whether ownership has been transferred to the IChunkReader/QueryChunkReaders; only close the raw chunks when the reader was not successfully constructed or ownership was not transferred. Otherwise let IChunkReader.close() perform the cleanup once.

@shuwenwei

Copy link
Copy Markdown
Member

Pipe aligned-chunk retry can reuse chunks already closed by a failed reader constructor

In TsFileInsertionEventScanParser.useNextPendingAlignedChunk() (around L915-L943), the pending group is removed only after AlignedChunkReader / AlignedSinglePageWholeChunkReader is successfully constructed. If construction throws:

  • AlignedChunkReader closes the passed time/value chunks internally.
  • AlignedSinglePageWholeChunkReader explicitly calls close() in its constructor catch, closing the same value chunks.

The pending group therefore still references value chunks whose page-AEAD references have already been released. If the exception is a classified memory-pressure exception, the code marks shouldRetryChunkHeaderAfterMemoryPressure and retries with that same closed group; otherwise later clearCachedAlignedChunkData() closes them again. This can both make the retry invalid and over-release the file crypto context.

Suggested fix: do not leave the original borrowed chunk list in pendingAlignedChunkGroups after a reader constructor has taken ownership and failed. Either construct with independently retained chunks and only release that attempt's references on failure, or remove/reload the pending group before retry so it never reuses closed chunks or closes them a second time.

@HTHou

HTHou commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@shuwenwei Thanks for checking the failure paths. Regarding the double-close comment, I checked both the sources and bytecode of the exact dependency used here, org.apache.tsfile:tsfile:2.5.1-261023-SNAPSHOT.

Chunk.close() delegates to EncryptParameter.close(), which calls destroy(). That method guards the release as follows (EncryptParameter.java:302–319):

synchronized (this) {
  if (destroyed) {
    return;
  }
  destroyed = true;
  // ...
}
if (fileEncryptContext != null) {
  fileEncryptContext.release();
}

Also, retain() returns a distinct EncryptParameter instance sharing the underlying file context. Each retained instance has its own destroyed flag.

So the constructor/registry cleanup and the loader's catch block can indeed both close the same chunk, but the second close does not decrement the shared reference count again. In this dependency version, repeated close is redundant cleanup; it does not over-release the context or invalidate other retained references.

This addresses the double-close/reference-count claim specifically. Reusing an already-closed object during a retry is a separate ownership question.

HTHou added 5 commits October 26, 2026 16:04
Retain independent aligned chunks and buffer views for each reader construction attempt so failed attempts do not invalidate pending groups. Release pending value chunk references after successful construction.

Propagate the recovered writer context to initial and snapshot recovery memtables. Cover constructor failure retries, partial retain cleanup, and aligned/non-aligned WAL insert and snapshot recovery.
@HTHou
HTHou merged commit 8affcb3 into apache:master Oct 26, 2026
46 of 48 checks passed
@HTHou
HTHou deleted the codex/tsfile-context-lifecycle branch October 26, 2026 23:04
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