feat: write gzipped metadata when the table asks for it - #389
Merged
Conversation
cedricziel
added a commit
to cedricziel/signaldb
that referenced
this pull request
Aug 6, 2026
…leanup The version hint names a *version*, not a file. Orphan cleanup rebuilt the current metadata file's path as `<hint>.metadata.json`, but the same version is `<hint>.gz.metadata.json` when a table sets `write.metadata.compression-codec = gzip`. The reconstructed path then matches nothing, the real file falls outside the live set, and orphan cleanup -- enabled by default with dry_run = false -- deletes the one file that must never be reclaimed: the live metadata pointer. Nothing sets that property today, so this is latent rather than live. It stops being latent the moment anything writes gzipped metadata, which the pinned iceberg-rust now can (JanKaul/iceberg-rust#389), or an external engine writes into the same table. Protect both spellings of the hinted version. The regression test has to defeat the detector's other guard first: the "never flag the newest version seen" fallback would shield the gzipped file whatever the hint did, so the test injects a higher-numbered orphan to take that protection, leaving the gzipped file dependent on the hint alone. Verified to fail without the fix.
`write.metadata.compression-codec` was half-implemented: readers have long decompressed a `*.gz.metadata.json`, but nothing ever wrote one, so the property did nothing and the decompression path was unreachable in practice. `metadata.json` is the largest artifact a busy table rewrites on every commit, and it is highly repetitive JSON, so leaving it uncompressed is a standing cost on every commit and every read. `new_metadata_location` now appends `.gz.metadata.json` when the table's properties name gzip, and `put_metadata` compresses when the location says so. The file name is the single source of truth for the encoding, which is already how the reader decides, so the two halves cannot disagree. Anything other than `gzip` -- including the spec's `none` default and an unrecognized codec -- writes plain JSON. Guessing at an unknown codec would produce a file named as something it is not, which no reader could open. Adds `TabularMetadataRef::properties`, since choosing the name needs them.
cedricziel
force-pushed
the
feat/metadata-gzip
branch
from
August 6, 2026 18:37
fe9e712 to
608a9ca
Compare
cedricziel
added a commit
to cedricziel/signaldb
that referenced
this pull request
Aug 6, 2026
…leanup The version hint names a *version*, not a file. Orphan cleanup rebuilt the current metadata file's path as `<hint>.metadata.json`, but the same version is `<hint>.gz.metadata.json` when a table sets `write.metadata.compression-codec = gzip`. The reconstructed path then matches nothing, the real file falls outside the live set, and orphan cleanup -- enabled by default with dry_run = false -- deletes the one file that must never be reclaimed: the live metadata pointer. Nothing sets that property today, so this is latent rather than live. It stops being latent the moment anything writes gzipped metadata, which the pinned iceberg-rust now can (JanKaul/iceberg-rust#389), or an external engine writes into the same table. Protect both spellings of the hinted version. The regression test has to defeat the detector's other guard first: the "never flag the newest version seen" fallback would shield the gzipped file whatever the hint did, so the test injects a higher-numbered orphan to take that protection, leaving the gzipped file dependent on the hint alone. Verified to fail without the fix.
cedricziel
added a commit
to cedricziel/signaldb
that referenced
this pull request
Aug 6, 2026
…onored (#1025) * feat(iceberg): size the trace bloom filters and pin the compression level Picks up JanKaul/iceberg-rust#387, which makes the Parquet writer honor the `write.parquet.*` table properties it previously ignored, and uses two of them. Compression: `CreateTableBuilder` records `zstd` / level 3 on every table, but the writer hardcoded level 1, so the metadata described a file that was never written. With the writer honoring the property, leaving it alone would silently move every write to level 3. Record level 1 -- what the files have actually been -- so the bytes are unchanged and the metadata is finally true. Raising it trades ingest CPU for storage and deserves a measurement, not an accident. Bloom filters: `trace_id` and `span_id` now carry an explicit `write.parquet.bloom-filter-fpp.column.<col> = 0.01`. A filter is sized from its target false-positive rate, and Parquet's 0.05 default reads one row group in twenty for nothing. For a single-trace lookup that wasted read is the whole query, so a five-fold cut is worth a filter ~40% larger. These are exactly the columns min/max statistics can never prune, which is why the filters exist at all. Both are creation-time properties, so they apply to new tables; existing tables keep what they were created with. * fix(compactor): protect a gzipped current metadata.json from orphan cleanup The version hint names a *version*, not a file. Orphan cleanup rebuilt the current metadata file's path as `<hint>.metadata.json`, but the same version is `<hint>.gz.metadata.json` when a table sets `write.metadata.compression-codec = gzip`. The reconstructed path then matches nothing, the real file falls outside the live set, and orphan cleanup -- enabled by default with dry_run = false -- deletes the one file that must never be reclaimed: the live metadata pointer. Nothing sets that property today, so this is latent rather than live. It stops being latent the moment anything writes gzipped metadata, which the pinned iceberg-rust now can (JanKaul/iceberg-rust#389), or an external engine writes into the same table. Protect both spellings of the hinted version. The regression test has to defeat the detector's other guard first: the "never flag the newest version seen" fallback would shield the gzipped file whatever the hint did, so the test injects a higher-numbered orphan to take that protection, leaving the gzipped file dependent on the hint alone. Verified to fail without the fix. * refactor(iceberg): stop repeating the pragmas the catalog now sets itself Upstream merged JanKaul/iceberg-rust#381 while #386 was in review: the SQL catalog now sets `journal_mode = wal` and `busy_timeout = 30000` on every SQLite connection by itself. Our session statements repeated both. Repeating them is harmless -- pragmas are idempotent -- but it hides which component owns the setting and would drift silently if the catalog ever changed them. Keep only what the catalog does not set: `synchronous = normal`, which under WAL skips an fsync per commit while staying crash-safe. Also repins to the integration branch rebased onto the new upstream/main, where our pool-options change composes with #381's defaults rather than replacing them: the catalog's pragmas run first, caller statements after, so a caller can add to them or override one. * fix(compactor): drop the removed revalidate_before_delete from the new test The gzipped-metadata regression test copied its `OrphanCleanupConfig` literal from a neighbouring test written before #1020 removed `revalidate_before_delete` (re-validation is now unconditional). CI caught it as E0560 -- the local package tests never did, because the field only appears in a `tests-integration` target. Also repins to the integration branch rebuilt on the current upstream/main: #383, #384 and #385 have merged upstream, so the branch now carries only the five still open (#382, #386, #387, #388, #389).
Contributor
Author
|
Trial close/reopen — Actions is triggering runs again after the incident; checking this one too. |
Owner
|
Thanks for PR. That's a great feature. I've made some additions to how the file catalog handles the gzip. |
The gzip metadata feature named files purely from `new_metadata_location`,
but the file catalog writes a temp file under that name and then copies it
to its own version-numbered name (`v{N}.metadata.json`), which never carried
the `.gz.` suffix. A gzipped table therefore stored gzip bytes under a plain
name, and the reader — which decides whether to decompress from the suffix
alone — would read them as raw JSON and fail. The version parsing also
choked on a `.gz.` name.
Derive the final name's suffix from the temp file actually written, so the
encoding of the two can never disagree, and teach `parse_version`, the
`latest_metadata` sort, and the file-system version-hint regex to accept the
`.gz.` variant.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JanKaul
force-pushed
the
feat/metadata-gzip
branch
from
August 11, 2026 15:08
e0f24bb to
cf2cf9e
Compare
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.
write.metadata.compression-codecwas half-implemented.parse_metadatahas long decompressed a*.gz.metadata.json, but nothing ever wrote one — so the property did nothing, and the decompression path was unreachable in practice.metadata.jsonis the largest artifact a busy table rewrites on every commit, and it is highly repetitive JSON, so leaving it uncompressed is a standing cost on every commit and every read.new_metadata_locationappends.gz.metadata.jsonwhen the table's properties name gzipput_metadatacompresses when the location says soThe file name is the single source of truth for the encoding — already how the reader decides — so the two halves cannot disagree.
Anything other than
gzip, including the spec'snonedefault and an unrecognized codec, writes plain JSON. Guessing at an unknown codec would produce a file named as something it is not, which no reader could open.Adds
TabularMetadataRef::properties, since choosing the name needs them. The version-hint regex already accepted thegz.variant.Tests: gzip round-trips through writer and reader (asserting the bytes are really a gzip stream and smaller than the JSON), a plain name stays plain, and codec recognition across casings and non-gzip values.