fix(cpp): never seal empty chunks and fix dangling ref in parallel tablet write - #909
Open
kkzi wants to merge 1 commit into
Open
fix(cpp): never seal empty chunks and fix dangling ref in parallel tablet write#909kkzi wants to merge 1 commit into
kkzi wants to merge 1 commit into
Conversation
…blet write Two fixes in TsFileWriter: 1. flush_chunk_group / flush_chunk_group_encoded: skip registered-but-empty measurement columns. A measurement that received no data in a window used to be sealed as an EMPTY chunk (count=0, dataSize=0). Java readers (TsFileSequenceReader self-check) treat such a file as crashed and refuse to load it. Mirror the aligned branch's existing hasData() check so empty columns never produce a chunk. 2. write_table (aligned parallel path): the submitted tasks run asynchronously on the thread pool, but the lambdas captured the loop variables (ctx, vt) by reference. Once the loop advances, every queued task reads the same / already-destroyed loop variable. Capture the per-iteration addresses by value instead.
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes two correctness issues in the C++ TsFile writer: avoiding creation of invalid empty chunks in the non-aligned flush path, and preventing dangling reference captures in the aligned parallel tablet write path.
Changes:
- Skip sealing registered-but-empty chunk writers in
flush_chunk_group()andflush_chunk_group_encoded()viaChunkWriter::hasData(). - Capture per-iteration
DeviceWriteCtx/ValueTaskpointers by value when submitting thread-pool tasks inwrite_table().
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes #908
Two fixes in
cpp/src/writer/tsfile_writer.cc1. Never seal empty chunks (non-aligned flush path)
flush_chunk_group()/flush_chunk_group_encoded()now skip registered-but-empty measurement columns via the existingChunkWriter::hasData()check — mirroring what the aligned branch already does.Before: a measurement that received no data in a window was sealed as an EMPTY chunk (
count=0, dataSize=0). Java readers (TsFileSequenceReaderself-check /TsFileSketchTool) treat the whole file as crashed and refuse to load it, even though the other chunks are valid.After: empty columns produce no chunk at all.
2. Fix dangling reference capture in parallel aligned tablet write
In
write_table()'s aligned parallel path, the tasks submitted to the thread pool previously captured the loop variables (ctx,vt) by reference. Since the pool executes tasks asynchronously (after the loop advances/exits), the references dangle and tasks can read wrong or out-of-scope state. The fix captures the per-iteration addresses by value (ctx_ptr/vt_ptr), so each task reads its ownDeviceWriteCtx/ValueTask.Verification
tsfile_writer.ccchanged, no build config touched).count=0chunk), andTsFileSketchTool/print-tsfile.batread it completely (END of TsFile,TsFile Sketch End, no errors).