[python] Track managed BLOB packs and delete their sidecars on abort. - #10114
Stephen0421 wants to merge 1 commit into
Conversation
The .blobref file lists packs referenced by one data file. Commit abort removes those sidecars and .row files, and leaves .blob packs in place when a BlobConsumer owns them. prepare_commit still keeps file ownership on the writer.
fb0080a to
e721d6f
Compare
| new_files=committed_files, | ||
| changelog_files=changelog_files, | ||
| total_buckets=self._runtime_total_buckets.get(partition), | ||
| preserve_blob_files_on_abort=preserve_blob_files, |
There was a problem hiding this comment.
This ownership flag is only present on the Python CommitMessage ; the Java-v14 serializer used by native commit does not encode it. With commit.native.enabled=true , TableCommit.abort() converts this message to a native message and calls native.abort() , so the native side receives no indication that the BlobConsumer owns these .blob packs and performs normal abort cleanup. That can delete data already handed to the consumer. The smallest safe fix is to make native commit/abort unsupported whenever any message has preserve_blob_files_on_abort=True, so abort falls back to _abort_commit_messages ; alternatively the native wire format and abort implementation must carry and honor this ownership state.
Please add a TableCommit.abort() test with native commit enabled and a preservation-marked message, asserting the native path is not used and the pack remains.
| end = offset + length | ||
| if end > len(data): | ||
| raise IOError("Truncated modified UTF-8 payload.") | ||
| return _decode_modified_utf8(data[offset:end]), end |
There was a problem hiding this comment.
non-blocking
The reused decoder does not validate that continuation bytes have the 10xxxxxx form. For example, a CRC-valid .blobref containing b"\xc0A" is accepted here and decoded as "\x01" , while Java DataInputStream.readUTF rejects that sequence. Since this format is intended to be Java-compatible, malformed files can be interpreted differently by Java and Python. Could we make the shared decoder strict (or validate locally), convert the decode failure to IOError , and add a CRC-valid malformed-continuation fixture that both implementations reject?
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED for the lifecycle part of this staged #9099 work. There is a concrete existing path: a data-evolution write creates a .row extra file, and aborting prepared commit messages previously removed the data file but left that sidecar. I reproduced a real table write with both files present, then TableCommit.abort(messages) removing both. The managed-BLOB reference collector is intentionally not yet wired into PK writes, so this PR does not by itself deliver .blobref generation; please keep the PR description and the follow-up dependency explicit.
Implementation: FINDINGS. [P1] Honor consumer-owned BLOB packs on the native abort route (paimon-python/pypaimon/write/file_store_write.py:339). The new preserve_blob_files_on_abort flag is read only by Python _abort_commit_messages. With commit.native.enabled=true, TableCommit.abort routes supported messages to native; native_messages_supported currently returns true even when this flag is set, and the Java abort deletes all new files, including .blob packs. A write using with_blob_consumer followed by native abort can therefore delete a pack whose descriptor the consumer retains. I verified the native eligibility check accepts such a message. Please route these aborts through the Python implementation or carry equivalent ownership semantics through the native protocol, and add a regression that exercises the actual TableCommit.abort dispatch. This blocks production merge for the opt-in native path.
Format/release boundary: Python's sidecar magic, version, modified UTF-8 and payload CRC match the existing Java reader in source and focused tests. The collector currently has no production collect_table call or DataFileMeta.extra_files registration; the later PK-write PR must provide that path and a Python-write/Java-read or GC reachability test before managed BLOBs can be enabled. No new on-disk format is produced by current PK writes in this PR.
Verification on head e721d6fb87: the three new Python modules passed 15/15; 13 targeted existing abort/multi-prepare tests passed; git diff --check passed. A real local data-evolution table write confirmed .parquet and .parquet.row exist before TableCommit.abort and neither exists after. The Java ManagedBlobReferenceFileTest passed 4/4 in a separate focused run. Native Rust runtime and an actual Python-to-Java sidecar round trip were not available locally; the native abort issue is established by the dispatch and Java deletion paths. Release gate: BLOCK until the native abort ownership path is fixed and covered.
Purpose
Part of the primary-key BLOB split (#9099), after the descriptor format (#9539), the read path (#9608), and keeping BlobViewStruct bytes when blob-as-descriptor is true (#10057).
.blobrefsidecar format andManagedBlobReferenceCollector. The on-disk layout matches JavaManagedBlobReferenceFile(magic, version 1, modified UTF-8, CRC of the payload only). The collector is a library and is not wired into the primary-key writer.extra_files(.blobref,.row). When a BlobConsumer owns the packs, paths ending in.blob(including.managed.blob) are left in place.prepare_commitstill returns a copy and leaves file ownership on the writer, sowriter.abort()deletes uncommitted data files. Batch HASH index files stay owned until abort; only stream prepare releases them.Not in this change:
BlobFormatWriter._copy_exactly, primary-key managed-BLOB writes, or the read/view path.Tests
managed_blob_reference_file_test(round trip, fixed Java-compatible fixture, truncated file, abort-then-close)managed_blob_lifecycle_test(sidecar deletion, consumer-owned packs preserved, resolved data file deleted when extras fail)managed_blob_write_ownership_test(prepare keeps writer ownership; BlobConsumer sets the preserve flag)test_blob_abort_deletes_uncommitted_files,test_vector_abort_deletes_uncommitted_files,test_vector_close_failure_after_prepare_raisestest_update_by_row_id_aborts_files_after_prepare_commit_failuretest_batch_writer_abort_after_prepare_deletes_hash_index,test_stream_writer_releases_prepared_hash_index_ownershiptest_multi_prepare_commit_ao,test_multi_prepare_commit_pk