[Feature] Support appending to a normally closed TsFile in C++ and C APIs #923

Activity

1. ColinLeeo

   added

   help wantedExtra attention is neededExtra attention is needed

   CPull requests that update C codePull requests that update C code

   c++Pull requests that update c++ codePull requests that update c++ code

   featureNew featureNew feature

   on Aug 27, 2026
2. Adarsh-Me commented on Sep 2, 2026

   

   Adarsh-Me

   on Sep 2, 2026

   More actions

   Hi! I'd like to take this issue. Could a committer please assign it to me? My plan is to design an append/reopen API in the C++ binding first, then mirror it in the C API, covering the normally-closed TsFile case with tests. Thanks!
3. ColinLeeo commented on Sep 3, 2026

   

   ColinLeeo

   on Sep 3, 2026

   ContributorAuthor

   More actions

   Thanks for your interest in working on this issue!
   The Java implementation can be used as a reference, especially RestorableTsFileIOWriter.getWriterForAppendingDataOnCompletedTsFile(File).

   The general workflow is to read the footer of a completed TsFile, locate and remove the tail metadata, recover the existing schema and chunk metadata, continue appending data, and regenerate the metadata when the writer is closed.

   You’re welcome to share some initial API and implementation design thoughts in this issue before getting started. Thanks!
4. ColinLeeo

   assigned

   Adarsh-Me

   on Sep 3, 2026
5. Adarsh-Me commented on Sep 23, 2026

   

   Adarsh-Me

   on Sep 23, 2026

   More actions

   Design for append-on-complete in the C++/C APIs. Nothing here is written yet — I'd like a sanity check on the recovery strategy (§3) and the boundary-marker finding (§5) before code.

   1. C++ surface

   One new method rather than an open() behaviour change:

   // storage/RestorableTsFileIOWriter
   // Opens a TsFile for appending. A normally-closed file has its metadata tail
   // removed in place; an incomplete file is recovered as today. Returns E_OK and
   // can_write() == true, or an error with the file left byte-identical.
   int open_for_append(const std::string& file_path);
   
   bool is_append_on_complete() const;   // complete file, tail removed

   open(path, truncate_corrupted) must keep returning can_write() == false for a complete file — test/file/restorable_tsfile_io_writer_test.cc:166 (OpenCompleteFile) pins can_write()==false, has_crashed()==false, get_truncated_size()==TSFILE_CHECK_COMPLETE, get_tsfile_io_writer()==nullptr. A separate entry point keeps every existing test untouched.

   has_crashed() should stay false on the append path. Today the recovery scan sets crashed_ = true unconditionally (restorable_tsfile_io_writer.cc:837), so that assignment needs to become mode-dependent; otherwise callers that use has_crashed() to decide whether data was lost will misread a clean append as a crash salvage.

   get_truncated_size() on the append path returns bytes removed (file_size - new_size), keeping the existing -2/0 sentinels for the incompatible/complete cases.

   2. C surface, and one data-loss trap

   TsFileWriter tsfile_writer_append_new(const char* pathname, ERRNO* err_code);
   TsFileWriter tsfile_writer_append_new_with_memory_threshold(
       const char* pathname, uint64_t memory_threshold, ERRNO* err_code);

   These must not be built on write_file_new() / TsFileWriter::open(path), because both truncate on open:

   • src/cwrapper/tsfile_cwrapper.cc:177 — int flags = O_RDWR | O_CREAT | O_TRUNC;
   • src/writer/tsfile_writer.cc:298 — return open(file_path, O_RDWR | O_CREAT | O_TRUNC, 0666);

   So a user who today tries to append through the existing C entry points loses the whole file before a writer object ever exists. The append entry points need their own non-truncating internal open (O_RDWR|O_CREAT|O_BINARY, no O_TRUNC, no O_EXCL), and the docs for write_file_new() should say plainly that it discards an existing file.

   Ownership: TsFileWriter::init(RestorableTsFileIOWriter*) sets io_writer_owned_ = false (tsfile_writer.h:197), so the C handle has to own the RestorableTsFileIOWriter and destroy it after the writer. The C++ side already has the right shapes — TsFileTableWriter(RestorableTsFileIOWriter*, ...) at src/writer/tsfile_table_writer.cc:27 and the tree equivalent at src/writer/tsfile_tree_writer.cc:36 — so the C work is mostly a context struct plus lifetime handling. The C API exposes no RestorableTsFileIOWriter at all today, so this is the first recovery entry point in the C layer.

   3. Recovery strategy: reuse the chunk scan, use the footer only to find the cut

   I checked the alternative — deserialize FileMetadata from the footer the way Java does — and I'd rather not. The scan already produces exactly what TsFileWriter::init(RestorableTsFileIOWriter*) consumes (tsfile_writer.cc:152-231): ChunkGroupMetas with real device IDs, per-measurement data_type_/encoding_/compression_type_, recovered statistics, aligned_devices_, and update_table_schema() + finalize_table_schemas() for the table model. Footer-driven recovery means writing a second recovery path over index-node structures, and it would have to re-derive per-chunk statistics that the scan already decodes from the pages themselves. Reusing the scan also inherits ~12 existing recovery tests (TableWriterRecoverAndWrite, AlignedTimeseriesRecoverAndWrite, MultiDeviceRecoverAndWriteWithTreeWriter, RecoveryAlignedSparseStatRespectsBitmap, …) instead of starting from zero.

   Algorithm, all before any mutation:

   1. fstat; size == 0 → existing empty-file path.
   2. Validate head magic + VERSION_NUM_BYTE (existing code, restorable_tsfile_io_writer.cc:609-628).
   3. Read tail magic. Absent → the file is already incomplete, fall straight into the existing recovery scan.
   4. Present → read meta_size as big-endian i32 at size - 10; require meta_size > 0 and meta_size + 11 <= size; cut = size - 10 - meta_size - 1.
   5. Validate the marker byte at cut (§5).
   6. Bounded recovery scan over [header_len, cut), which must consume exactly up to cut.
   7. Only now truncate(cut), seek_to_end(), can_write_ = true.

   Step 6 is where this differs from the crash path on purpose. Crash recovery is best-effort salvage: it truncates at the last parseable offset, which is the right call for a half-written file. Here the file is known-good, so partial progress is a failure: any early stop returns an error and leaves the file alone. Without that rule, a file whose chunks the scanner can't parse gets truncated at step 7 down to ~7 bytes, i.e. append destroys a file it should have refused.

   Two things already work in our favour:

   • OPERATION_INDEX_RANGE is handled by the scan (restorable_tsfile_io_writer.cc:721-737), which matters because end_file() writes that segment before the separator (tsfile_io_writer.cc:305-307), so truncation leaves the previous one in place and each append/close cycle adds another. The 1+16 skip means they are consumed cleanly rather than tripping the unknown-marker branch at :814. I want an explicit test for repeated append→close→append→close cycles, since accumulation is only benign if it stays benign at 3+ cycles.
   • Out-of-order rejection (requirement 9) is already wired: init(RestorableTsFileIOWriter*) sets enforce_recovered_last_time_order_ (tsfile_writer.h:198-202) and it is checked at ~12 write/flush sites. Nothing new to design.

   Existing-table semantics need no decision either: register_table() already returns E_ALREADY_EXIST when the name is present (tsfile_writer.cc:261-272), so "evolve an existing table or aligned schema" = rejected, and appending to a recovered table works without re-registering. New device/new non-aligned measurement/new table after recovery all go through the ordinary register_* paths. Preserving the latest codec per series when a series was written with different codecs across chunks is already handled at tsfile_writer.cc:217-225.

   4. Encryption

   There is no encryption in cpp/src at all — no EncryptParameter type and no cipher path anywhere in the tree. The writer only records properties, and the spot where encryption would live is still a TODO: src/file/tsfile_io_writer.h:76-78 reads if (common::g_config_value_.encrypt_flag_) { // TODO: support encrypt … encrypt_level_ = "2"; So an encrypted file is an unreadable file for this library, and open_for_append() should fail with E_TSFILE_CORRUPTED (C: RET_TSFILE_CORRUPTED, 35) rather than truncate. The bounded-scan rule in step 6 above is what actually guarantees that, since encrypted chunk bytes will not parse. I'd document encrypted input as unsupported rather than designing a rejection code for it.

   5. Boundary-marker finding — needs a maintainer answer

   The byte at cut is SEPARATOR_MARKER (0x02) in both current writers — tsfile_io_writer.cc:600-603 and TsFileIOWriter.java:510-514 (Java records metaOffset = out.getPosition() before writing the separator, so Java's truncate(fileMetadataPos - 1) and our cut land on the same offset; I verified the two loadMetadataSize/write_separator_marker definitions agree at byte level).

   It is not 0x02 in the fixtures checked into this repo. I measured all five, big-endian i32 at size-10:

   file	size	meta_size	cut	byte at cut
   cpp/examples/test_cpp.tsfile	447	242	194	0x03
   python/tests/resources/simple_table_t1.tsfile	2965	351	2603	0x03
   python/tests/resources/simple_table_t2.tsfile	2677	351	2315	0x03
   python/tests/resources/simple_tree.tsfile	842	174	657	0x03
   python/tests/resources/table_with_time_column.tsfile	644	259	374	0x03

   All five also carry head magic TsFile + version byte 0x04 and a matching tail magic, so the arithmetic (meta_size covers the metadata body only, separator is 1 byte before it) holds; only the marker's identity differs. MetaMarker.VERSION = 3 is @Deprecated in Java and nothing in java/tsfile/src/main/java writes it any more, so my guess is these fixtures predate the current tail, but I don't know their provenance and I'd rather not encode a guess. Two questions:

   1. Should open_for_append() accept 0x03 at the boundary as well, or reject anything that isn't 0x02? My default is reject-with-error, which is safe for files this library writes and conservative for everything else.
   2. Do we want a "compatible enough" gate on the version byte beyond the exact VERSION_NUM_BYTE == 0x04 equality check, given VERSION_NUMBER_V3 = 0x03 exists in Java and v3 files may still be in users' hands?

   The related "large-metadata" test case: I would not copy the reader's ASSERT(tsfile_meta_size > 0 && tsfile_meta_size <= (1ll << 20)) (tsfile_io_reader.cc:555). ASSERT compiles to assert() unless NDEBUG (src/utils/util_define.h:139-142), so on a malformed tail that aborts the process instead of returning an error code — bad for a function whose whole job is to validate untrusted file bytes — and the 1 MiB ceiling would also reject the legitimately-large-metadata file the issue asks us to cover. The append path uses returned error codes and a size bound expressed against the file length only.

   6. Test plan

   C++ (test/file/, next to the existing recovery tests; each is the complete-file twin of an existing truncated-file test): tree append and verify old+new rows; table append; new device and new non-aligned measurement after recovery; aligned-series and existing-table registration rejected with the documented code; timestamps older than the recovered per-device last_time_; no-op append+close then a full read-back diff of every series' statistics — that one is the real guard, because the rewritten tail is regenerated from scanned chunk-level metadata, not copied from the original footer, so a silent loss there needs to be caught rather than assumed; repeated append/close cycles for the OPERATION_INDEX_RANGE accumulation; empty, header-only, bad-magic, bad-version, meta_size out of range, and separator-mismatch files all leaving the input byte-identical; plus the C wrapper round-trip, including that the append entry point does not truncate.

   Cross-implementation check I'd like to add: read a C++-appended file back with the Java reader, so the regenerated tail is proven acceptable to the reference implementation and not just to the code that wrote it.

   7. What I have and have not verified

   Verified: the file-format arithmetic and every source claim above, by reading the code at the cited lines, plus the byte measurements in §5 run against the five checked-in fixtures.

   Not verified: I have not compiled or run anything. This host has no cmake/ninja and no configured MSVC/MinGW build of libtsfile, so no test in §6 has executed, and I am not claiming any behaviour I have not read from source. First PR will therefore be the truncation/validation step plus one round-trip test, run locally, before the C wrapper lands.
6. ColinLeeo commented on Sep 30, 2026

   

   ColinLeeo

   on Sep 30, 2026

   ContributorAuthor

   More actions

   Sorry for the delayed response, and please excuse the delay. I carefully reviewed your implementation, and it looks very good. I also compared the proposed flow with the Java implementation.

   Overall, the design is aligned with Java’s completed-file append workflow. In Java, RestorableTsFileIOWriter.getWriterForAppendingDataOnCompletedTsFile(File) detects a complete file, uses the metadata size in the footer to locate fileMetadataPos, removes the existing metadata tail, and then enters the normal RestorableTsFileIOWriter recovery flow so that new data can be appended and a new footer can be generated on close.

   Therefore, I agree with adding a separate open_for_append() entry point instead of changing the behavior of the existing open(). The current open() semantics can remain unchanged, with a normally completed file remaining non-writable, while the new entry point handles the explicit append workflow.

   For the marker at the metadata boundary, I suggest following the actual Java behavior. The Java append helper calculates the truncation position from the footer metadata size and calls truncate(fileMetadataPos - 1) without requiring the boundary byte to be a specific marker. Therefore, I do not think the C++ implementation needs to introduce a stricter marker requirement than Java.

   Regarding the file version, Java currently supports versions V3 through V4, while the current C++ implementation only supports V4. I suggest that this issue should first align the C++ completed-file append workflow with Java while keeping the current C++ version scope limited to V4. V3 compatibility can be handled as a separate follow-up task, since it would require complete support in the corresponding chunk, page, metadata, and configuration deserialization paths.

   The ownership and lifetime handling in the C API also look clear. The recovered RestorableTsFileIOWriter needs to remain alive for the entire lifetime of the higher-level tree/table writer and should only be released after the higher-level writer has completed its close operation. The proposed context/lifetime design should be sufficient for this.

   Overall, I agree with the current design and think it is ready to move forward. The scope can remain:

   • align with Java’s completed-file append workflow;
   • add a dedicated open_for_append() API;
   • preserve the existing open() behavior;
   • support V4 only for now;
   • manage the recovered writer lifetime correctly in the C API;
   • add coverage for completed-file append, repeated append/close cycles, no-op append, malformed footers, and C-wrapper lifetime handling.
7. Adarsh-Me commented on Oct 10, 2026

   

   Adarsh-Me

   on Oct 10, 2026

   More actions

   Thanks - the marker point holds: open_for_append cuts at file_size - (MAGIC_LEN + 4), length read from the footer, never inspecting that byte, so nothing is stricter than Java (L745-760). open() keeps the semantics you described, scope stays V4-only with V3 a follow-up. #980's append path is red; once fixed I'd land the C API as a second PR. Sound?
8. ColinLeeo commented on Oct 11, 2026

   

   ColinLeeo

   on Oct 11, 2026

   ContributorAuthor

   More actions

   Sounds good, thanks! Let’s get #980’s tests passing first, then add the C API in a separate PR.