Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This draft extends
feature/trusted-index-modewith the resource-lifetime and identity contracts needed by large, multi-process training workloads:mmapon Linux;DatasetRuntimethe explicit owner of index mappings, Reader sessions, prepared-series handles, the query executor, and their leases;fork()paths;The changes are confined to the Python Dataset API. They do not add TimerLathe-specific imports, change the C++ reader ABI, implement native fused multi-window I/O, or claim end-to-end Arrow parity.
Downstream integration: https://github.com/thulab/TimerLathe/pull/6
Base and scope
feature/trusted-index-mode@4159c635de8f29c973f56d06a11771c8c0121ff9gewu/timer4-batched-window-read-clean@94649f3e4a1d0f1245cf0616ac24c3853db2769ae9f28e09— release the Dataset Index file aftermmap;3d21b820— bound runtime resource ownership and hardenfork()/failure cleanup;94649f3e— add trusted index/data attestations and generation identities.Motivation
The motivating downstream workload opens a fixed TimeBench asset containing 218 logical datasets from 8 distributed ranks with multiple DataLoader workers per rank. The dataset is immutable for the duration of training and already has trusted
.tsidxfiles. At this scale, a resource cost that looks small for oneTsFileDataFramebecomes multiplicative:The existing trusted-index mode removes expensive validation work, but it did not yet define a complete handoff from a trusted parent process to workers, nor an explicit lifecycle for every native/index resource. The concrete failure modes were:
mmapimplementation;fork()could inherit locks, thread-pool state, Readers, prepared handles, and leases that are unsafe to reuse in the child;This PR treats those as one ownership problem: expensive identity work happens once at the trust boundary; each process owns only its own mutable/native runtime; every retained object has an explicit cap, lease, and deterministic teardown path.
Design overview
flowchart LR P[Trusted parent] --> IH[Hash finalized .tsidx once] P --> DF[Open and fstat data files] IH --> IA[IndexAttestation] DF --> DA[Ordered DataFileAttestations] IA --> W[Worker open] DA --> W W --> V[Canonical path + stat validation] V --> M[Read-only mapped index] M --> R[Bounded ReaderSessionPool] M --> C[Bounded PreparedSeriesCache] R --> Q[Queries] C --> QThe parent hashes index bytes only once. Workers validate canonical path and stable file metadata, map the attested index, and create fresh process-local Reader/cache/executor state. Mismatch fails before value I/O.
Detailed changes
1. Drop the original
.tsidxfile descriptor aftermmapMappedDatasetIndexnow initializes every owned resource toNone, creates the read-only mapping, and closes the original file object immediately on Linux. The mapping and itsmemoryviewremain live, so index access is unchanged.The important distinction is:
open(..., "rb")descriptor is released;mmapobject still owns one descriptor on the tested runtime;MappedDatasetIndex.close();_close_owned_resources()uses nestedfinallyblocks, is idempotent, and continues cleaning later resources even if an earlier release raises. Constructor failures before/after mapping use the same path.This is intentionally narrower than a “zero-FD mmap” or an end-to-end zero-copy claim.
2. Explicit runtime leases and quiescent teardown
DatasetRuntimenow owns:MappedDatasetIndex;ReaderSessionPool;PreparedSeriesCache;ThreadPoolExecutor;TsFileDataFrame/Timeseries views.RuntimeLeaseprotects an object/view lifetime._QueryLeaseprotects an active query. When the last object lease closes, the runtime stops accepting work, waits for active query leases to drain, and tears resources down exactly once.This prevents a subset view or result path from closing native state still in use by another view. It also gives close/failure code a single ownership graph instead of several loosely related dictionaries.
3. Hard-bounded Reader sessions
ReaderSessionPoolis a per-runtime LRU with an explicitmax_open_filescap:close()waits for active uses to finish and then closes all Readers;The existing
TSFILE_DATAFRAME_MAX_OPEN_FILESsetting controls this cap (default 16 inDatasetRuntime). This is a per-runtime/per-DataFrame bound, not yet a global per-process FD budget. Applications retaining many DataFrames must still size the product of frame count and per-frame Reader cap.4. Bounded, generation-aware
PreparedSeriesCacheThe prepared-series cache is now a runtime-wide, single-flight LRU:
TSFILE_DATAFRAME_MAX_PREPARED_SERIESsets a non-negative retention cap;0is a supported “no idle retention” mode;BaseException, decode failure, partial construction, and close failure paths release leases and continue cleanup.With attestations, cache keys use
file_generation_token(index_identity, data_manifest_identity, file)plus locator ID. Without attestations, the previous mapped-index/file metadata identity remains the compatibility fallback.5. Fork-safe process-local runtime replacement
Locks, native Readers, prepared handles, and executor threads are not reused across
fork().TsFileDataFrame._ensure_process_local_runtime()creates a fresh child runtime on first use;This targets PyTorch/DataLoader-style worker creation while keeping normal single-process behavior unchanged.
6. Public trusted-attestation API
The new generic API exported from
tsfile.datasetis:IndexAttestation.tsidxDataFileAttestationattest_index()validate_index_attestation()attest_data_file()fstatvalidate_data_file_attestation()data_manifest_identity()file_generation_token()AttestationMismatchErrorTsFileDataFrameandDatasetRuntimeacceptindex_attestationanddata_file_attestationsonly as a complete pair and only withtrust_index=True. In the attested path:0..N-1in exact order;The encoding uses explicit domain separators, lengths, little-endian
uint64fields, and a schema version so it does not depend on pickle, locale, mapping order, or Python object identity.Trust and security boundary
Attested trusted mode is for a prebuilt, immutable dataset managed by the application. It does not turn untrusted input into trusted input and does not continuously monitor files after open.
The intended contract is:
Safe mode remains the default. Callers that do not pass attestations keep existing path expansion, index matching, validation, and generation checks.
Observable effects
Resource behavior proven by tests
0, while active/dependent entries remain valid.fork().Performance interpretation
This branch removes repeated identity work from workers and bounds resource growth, but no standalone wall-time benchmark has yet isolated these three commits. It would be misleading to assign the downstream training speedup to this PR alone.
In the downstream Timer4 experiment tree, the larger composed optimization stack observed a short-run data-handoff P50 change from 886.022 ms to 2.348 ms, P95 from 957.214 ms to 4.701 ms, and throughput from 2,835.81 to 4,160.13 samples/s. Those numbers:
They are motivation for further profiling, not a benchmark claim for this PR.
Compatibility and non-goals
trust_index=Truecallers without attestations continue to work.query_windows_many()/merged chunk-page read is implemented.map_query_groups()still schedules independent logical queries; it is not fused physical I/O.Validation
Fresh verification on Linux/Python 3.11:
python/testssuite: 293 passed;git diff --checkpasses;Tests cover FD/VMA attribution via
/proc, idempotent close, mmap and post-mmap failures, active Reader eviction, cache plateau/single-flight/dependency ordering,BaseExceptioncleanup, process-local reopen afterfork(), attestation mutation/mismatch, canonical encoding, Python 3.9-compatible imports, and public API exposure.Suggested follow-up work
This PR intentionally establishes a safe baseline that TsFile maintainers can profile and extend. The highest-value next steps are:
2.5.0.dev0wheel that does not contain these APIs.ReadFile::read()path still copies mapped bytes into a caller buffer. End-to-end zero-copy requires explicit borrowed-span ownership from file mapping through page parsing/decompression/decoding, with safe mapping lifetime and mutation rules.Review guide
The most important review questions are:
fork()?