mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-20 13:04:25 +02:00
eea3f232da067c8eb8bde13bb06dbdb5f7da3b20
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris LuandDevin
|
50a9312e1e |
iceberg maintenance: v3 row lineage so v3 tables compact and rewrite manifests (#11653)
* iceberg maintenance: commit v3 row lineage on the three commit paths
iceberg-go 0.7.0 refuses every snapshot on a format-version 3 table
that lacks first-row-id and added-rows ("invalid row lineage"), so no
v3 table was ever compacted or had its manifests rewritten; worse,
passing 0 to the manifest list would have handed new files row IDs
existing rows already hold.
Each commit now plans first-row-id from the table's next-row-id, lets
the v3 manifest-list writer assign manifest first_row_id from there,
and reports the assigned range as the snapshot's added-rows so
next-row-id advances by exactly that. The commit guard also treats a
moved next-row-id or a changed format version as a stale plan, like a
moved head: a commit to another branch advances next-row-id too, and
manifests written for the old version must not be committed.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: keep row identity through v3 compaction
A rewrite that moves rows into a new data file must not renumber them.
On v3 tables the merge now appends _row_id and
_last_updated_sequence_number (spec field ids 2147483540/2147483539)
to the output and fills each row with the value its input
materializes, else the inherited one: the input file's first_row_id
plus the row's position before any delete, and the manifest entry's
data sequence number. Inputs are matched with their lineage columns
set aside, so a file an earlier compaction wrote merges with one an
engine wrote; a duplicate lineage column or a reserved name under
another field id fails the bin instead of writing a doubled column.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: skip rewrite_position_delete_files on v3 tables
The spec forbids adding position-delete files to a format-version 3
table (v3 writes deletion vectors), and this rewrite adds new ones.
Detection no longer proposes the operation and the operation returns a
skip result; compaction still applies and removes existing position
deletes.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: keep Puffin deletion vectors out of position-delete reads
A Puffin file in a delete manifest is a deletion vector, not a
position-delete file: reading it as Parquet fails, and merging the data
file it covers without applying the vector resurrects deleted rows.
Skip Puffin entries in the position-delete collection and the delete
rewrite group scan, and exclude the files a vector covers from
compaction candidates - in compactDataFiles and in eligibility
detection alike. A vector whose referenced_data_file is missing or
unresolvable fails the run rather than silently compacting whatever it
points at.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: name a merged file's own first row ID
Rewritten rows keep their IDs, so a merged data file's first_row_id is
the _row_id of its first written row - not the next free range the
manifest list would otherwise assign to the entry at commit. Read the
first materialized value back off the merged output and stamp it on the
manifest entry.
Add an end-to-end test that populates a v3 table whose entries carry no
first_row_id, so the compact path exercises manifest first_row_id
inheritance through the stored manifest list, then commits and reads
the result back through the real manifest readers.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: locate the _row_id column by index, not row position
Repeated columns shift parquet.Row positions, so leaf.ColumnIndex is not
the value's slot — a first row containing a list or map saved an unrelated
value as the merged file's first_row_id.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* iceberg maintenance: read the returned row before treating EOF as empty
parquet-go can return a row alongside io.EOF; a one-row merged file then
lost its first_row_id and the manifest list would allocate a fresh range.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test: avoid nil dereference in first-row-id failure message
---------
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
||
|
Chris LuandDevin
|
0305e837fd |
iceberg maintenance: keep compacted files prunable (stats, bound order, row groups) (#11654)
* iceberg maintenance: record column statistics on compacted files A compacted file's manifest entry was built with no column_sizes, value_counts, null_value_counts, lower_bounds, upper_bounds or split_offsets, so no reader could skip a compacted file on any predicate. parquet-go already writes exact per-chunk min/max and null counts into the footer; read that footer back after the merge and record it on the data file, bounds as the spec's single-value serialization with string and binary truncated as truncate(16). A column whose bounds cannot be converted exactly gets none, and a statistics failure is logged while the compaction commits anyway: metrics are an optimization, not a correctness requirement. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg maintenance: merge bins in bound order so compacted files stay prunable A bin's files were concatenated in manifest order, or largest-first when a partition was split under the target size, so inputs disjoint on a column came out as outputs that overlapped on it. Order each bin's files by their bounds on one column before merging and split an oversized partition into runs of consecutive files, so every output covers one contiguous range. The column is the first identity field of the table's sort order when it declares one (a descending order sorts by upper bound), otherwise the first schema column every candidate file has bounds for; detection resolves the same order so it plans the bins execution builds. Files without bounds keep the old behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg maintenance: cap compacted files' row groups from table config Neither merge writer set a row-group limit (parquet-go's default is unlimited rows), so every compacted file was a single row group and readers could not skip inside it either. Rows per row group now come from the table's write.parquet.row-group-limit and write.parquet.row-group-size-bytes, defaulting to PyIceberg's 1 048 576 rows and Iceberg's 128 MiB, with the byte size turned into rows from the bin's inputs' compressed bytes per row and a floor of 1 024 rows. Both writers take the cap, and the statistics the entry records list one split offset per row group. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg: keep compaction order eligibility per group, rescue stranded runs The merge order resolved over all candidates, so one oversized or non-Parquet file without bounds disabled ordering for files that could participate. Resolve it per partition group over the eligible entries. Ordered runs too short to merge were dropped entirely. Runs from an inferred bounds order now fall back to size-based packing — ordering is a preference there — while runs under a declared sort order are still left for later passes so the sort contract holds. An explicit write.parquet.row-group-limit is a cap, not a floor: values below the estimate floor are now honored instead of being raised. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg: only use the declared sort order when its bounds are complete An entry without bounds on the sort column sorted to the tail and merged into an output claiming an order it cannot verify. Fall back to bound inference instead of ordering by a later sort field alone. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg maintenance: keep ordered runs when the full repack yields nothing The bestEffort leftover fallback removed the ordered runs before checking whether repacking the whole bin produced any bins, discarding valid compaction work. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
97b54adcf6 |
iceberg: sort compaction bins on disk instead of in memory (#11112)
A sorted rewrite collected every row of a bin into one slice and sorted it there, so a bin larger than the worker's heap could not be sorted at all. sort_max_input_mb existed for that reason and skipped the bins it capped. parquet-go's SortingWriter buffers sort_buffer_rows rows, encodes each buffer as a sorted run, and merges the runs at close; backing those runs with a FileBufferPool keeps them in files rather than on the heap. sort_spill_dir says where, defaulting to the system temp directory — NewFileBufferPool resolves an empty path to the working directory, which is not what an unset setting means. The output now also declares its sorting columns, which the plain writer the sorted path used never did. Claude-Session: https://claude.ai/code/session_015SZkLTUvd1svDu4xdr6Q3y |
||
|
Dmitriy PavlovandChris Lu
|
1d0b97f4c6 |
avro: field time.Time <> iceberg.date (#11091)
* iceberg: normalize foreign day partitions during manifest rewrite * test: cover manifest rewrite with foreign day partitions * iceberg: restore every foreign partition value, not just day transforms iceberg-go takes a partition field's logical type from the last branch of its Avro union, so a writer that spells an optional partition [<type>, null] rather than [null, <type>] leaves the value as whatever the Avro decoder produced. A day or date partition then arrives as a time.Time the manifest writer cannot encode, and a time partition is worse: time.Duration converts to int64 nanoseconds and silently records the wrong value. ReadManifest sits next to ReadManifestList, the other shim for what foreign writers put on the wire, and converts each partition value back to the Iceberg representation for its field type. Claude-Session: https://claude.ai/code/session_01FdQyRuWF9SuCnPn21iH9yR * iceberg: read manifests that carry partition values through the shim Compaction, delete rewrite and their detection passes read entries and write the same partition values back into new manifests, so they fail on a foreign day partition exactly as manifest rewrite does. Where filters see it too: literalMatchesActual falls through to fmt.Sprint, so a time.Time renders as a timestamp and never matches the day the user asked for. The two remaining readers, orphan collection and the admin preview, only look at file paths and stay on iceberg.ReadManifest. Claude-Session: https://claude.ai/code/session_01FdQyRuWF9SuCnPn21iH9yR * iceberg: convert partition values before the writer rebinds logical types Dimonyga checked the manifests of a live Doris table: every input spells the partition union null-first, with the date logical type present, so the union ordering is not what breaks the merge. The conversion is lazy. iceberg-go converts what the Avro decoder returned on the first Partition() call, using the logical types read from the manifest being parsed, and ManifestWriter.addEntry rebinds them to the manifest it is about to write before it makes that call. A day partition is where the two disagree -- iceberg-go's day transform reports an int32 result type, so the manifest it writes carries no date logical type at all -- and an entry nobody looked at in between converts against that and keeps its time.Time. That is why only rewrite_manifests failed: compaction and delete rewrite group entries by partitionKey(df.Partition()) first, which converts them, and a where filter does the same. Reading every entry's partition here converts them all while the manifest's own logical types are still in place. Claude-Session: https://claude.ai/code/session_01FdQyRuWF9SuCnPn21iH9yR --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
4500bdf88e |
iceberg: accept lowercase parquet file format when planning compaction (#10751)
* iceberg: accept lowercase parquet file format when planning compaction * iceberg: expect absolute added-file paths in compaction integration test |
||
|
Chris Lu
|
4b0d09683a |
iceberg: read manifest lists that omit the Avro format version (#10475)
* s3tables: read Iceberg manifest lists that omit the Avro format version The Iceberg spec pins the Avro header metadata of manifest files but says nothing about manifest lists, so writers disagree. Java and PyIceberg record "format-version"; DuckDB writes no header metadata at all. iceberg-go reads a missing entry as v1, so every v2 manifest listed in a DuckDB-written list is rejected with manifest file's 'format-version' metadata indicates version 2, but entry from manifest list indicates version 1 and, because v1 has no "content" field, delete manifests silently decode as data manifests. ReadManifestList derives the version from the record schema the writer embedded - v2 added "content" and the sequence numbers, v3 added "first_row_id" - and splices it into the header before handing the bytes to iceberg-go. Lists that already carry the entry, and input that is not a parseable Avro container, go through untouched. * iceberg: parse DuckDB-written manifest lists in maintenance and data preview Every manifest list read - the four maintenance operations and the admin table data preview - went straight to iceberg-go, so tables written by DuckDB failed detection and all of compact, remove_orphans, rewrite_manifests and expire_snapshots before they touched anything. Route them through s3tables.ReadManifestList, which recovers the format version the writer left out of the Avro header. This also restores the manifest content type on those tables: with the list read as v1 every delete manifest looked like a data manifest, which hid deletes from the compaction guard and made the preview report a table with position deletes as having none. |
||
|
Chris Lu
|
1e1b2bb2f9 |
iceberg maintenance: record file counters on the snapshots it commits (#10420)
A maintenance snapshot carried only its own labels — merged-files, delete-groups and friends — and no summary counters, so every engine that reads a table's size out of the current snapshot summary reported nothing for it: PyIceberg's inspect.snapshots, Trino's $snapshots and Spark's DESCRIBE all read total-records, total-data-files and total-files-size verbatim, and a table lost them the moment compaction touched it. Accumulate the files each operation adds and removes, and render them the way the spec defines: the added-*/removed-* counters from the files themselves, then the totals carried over from the parent snapshot. Carry a total only when the parent recorded it. Iceberg treats a missing total as zero, which turns a compaction replacing two files with one into a negative total-data-files, or a table with millions of rows into total-records: 0. Leaving the field out lets a reader fall back to the manifests instead of believing a made-up number. Compaction also accounts for the delete files it consumes, so a run that folds every delete into the rewritten data reports them as removed. |
||
|
Chris Lu
|
b4b0346f95 |
iceberg maintenance: resolve table files from the recorded location (#10418)
The worker assumed every file of a table sits under its catalog path, so loadFileByIcebergPath stripped the scheme off a recorded location and joined the remainder onto /buckets/<bucket>/<ns>/<table>. A table the REST catalog placed elsewhere in the bucket — which is what a client gets whenever the catalog path is already occupied — then resolves to a doubled path: lookup /buckets/lake/source/t/lake/source/t-0cd81bca-.../metadata/snap-.avro so the very first manifest list read fails and the job fails again on every scan interval, indefinitely. Resolve absolute references (s3:// URIs and /buckets paths) from the bucket root and keep relative ones under the table's own directory; the bucket-relative form is now the canonical key everywhere references are compared. That directory comes from the metadata location the catalog stores, so reads, writes and deletes all land where the table's other files are instead of splitting it across two trees. References outside the table's bucket are rejected rather than silently misresolved. Rewritten position-delete files now name their data file by absolute URI, the way the table itself names it, instead of a path relative to the table. |
||
|
Chris Lu
|
07c9e0db85 |
iceberg maintenance: write absolute s3:// locations into table metadata (#10363)
The maintenance operations built manifest, manifest-list, data-file and
metadata-log paths with a bare path.Join("metadata", ...), so a single
maintenance run wrote scheme-less relative paths into the new snapshot,
the metadata-log and the table xattr. SeaweedFS reads those back fine
because normalizeIcebergPath accepts both forms, but strict readers
resolve every location through S3FileIO and fail with "Invalid S3 URI,
cannot determine scheme", leaving the whole table unreadable after any
maintenance commit.
Add absoluteIcebergPath, the inverse of normalizeIcebergPath, and apply
it at every site that authors locations: rewrite_manifests, compact,
rewrite_position_delete_files, and the shared commit path. The base is
derived from the bucket and table path since that is where the worker
physically writes, matching the locations the REST catalog generates.
|
||
|
Chris Lu
|
1f6f473995 |
refactor(worker): co-locate plugin handlers with their task packages (#9301)
* refactor(worker): co-locate plugin handlers with their task packages
Move every per-task plugin handler from weed/plugin/worker/ into the
matching weed/worker/tasks/<name>/ package, so each task owns its
detection, scheduling, execution, and plugin handler in one place.
Step 0 (within pluginworker, no behavior change): extract shared helpers
that previously lived inside individual handler files into dedicated
files and export the ones now consumed across packages.
- activity.go: BuildExecutorActivity, BuildDetectorActivity
- config.go: ReadStringConfig/Double/Int64/Bytes/StringList, MapTaskPriority
- interval.go: ShouldSkipDetectionByInterval
- volume_state.go: VolumeState + consts, FilterMetricsByVolumeState/Location
- collection_filter.go: CollectionFilterMode + consts
- volume_metrics.go: export CollectVolumeMetricsFromMasters,
MasterAddressCandidates, FetchVolumeList
- testing_senders_test.go: shared test stubs
Phase 1: move the per-task plugin handlers (and the iceberg subpackage)
into their task packages.
weed/plugin/worker/vacuum_handler.go -> weed/worker/tasks/vacuum/plugin_handler.go
weed/plugin/worker/ec_balance_handler.go -> weed/worker/tasks/ec_balance/plugin_handler.go
weed/plugin/worker/erasure_coding_handler.go -> weed/worker/tasks/erasure_coding/plugin_handler.go
weed/plugin/worker/volume_balance_handler.go -> weed/worker/tasks/balance/plugin_handler.go
weed/plugin/worker/iceberg/ -> weed/worker/tasks/iceberg/
weed/plugin/worker/handlers/handlers.go now blank-imports all five
task subpackages so their init() registrations fire.
weed/command/mini.go and the worker tests construct the handler with
vacuum.DefaultMaxExecutionConcurrency (the constant moved with the
vacuum handler).
admin_script remains in weed/plugin/worker/ because there is no
underlying weed/worker/tasks/admin_script/ package to merge with.
* refactor(worker): update test/plugin_workers imports for moved handlers
Three handler constructors moved out of pluginworker into their task
packages — update the integration test files in test/plugin_workers/
to import from the new locations:
pluginworker.NewVacuumHandler -> vacuum.NewVacuumHandler
pluginworker.NewVolumeBalanceHandler -> balance.NewVolumeBalanceHandler
pluginworker.NewErasureCodingHandler -> erasure_coding.NewErasureCodingHandler
The pluginworker import is kept where the file still uses
pluginworker.WorkerOptions / pluginworker.JobHandler.
* refactor(worker): update test/s3tables iceberg import path
The iceberg subpackage moved from weed/plugin/worker/iceberg/ to
weed/worker/tasks/iceberg/. test/s3tables/maintenance/maintenance_integration_test.go
still imported the old path, breaking S3 Tables / RisingWave / Trino /
Spark / Iceberg-catalog / STS integration test builds.
Mirrors the OSS-side fix needed by every job in the run that
transitively imports test/s3tables/maintenance.
* chore: gofmt PR-touched files
The S3 Tables Format Check job runs `gofmt -l` over weed/s3api/s3tables
and test/s3tables, then fails if anything is unformatted. Files this
PR moved or modified had import-grouping and trailing-spacing issues
introduced by perl-based renames; reformat them with gofmt -w.
Touched files:
test/plugin_workers/erasure_coding/{detection,execution}_test.go
test/s3tables/maintenance/maintenance_integration_test.go
weed/plugin/worker/handlers/handlers.go
weed/worker/tasks/{balance,ec_balance,erasure_coding,vacuum}/plugin_handler*.go
* refactor(worker): bounds-checked int conversions for plugin config values
CodeQL flagged 18 go/incorrect-integer-conversion warnings on the moved
plugin handler files: results of pluginworker.ReadInt64Config (which
ultimately calls strconv.ParseInt with bit size 64) were being narrowed
to int32/uint32/int without an upper-bound check, so a malicious or
malformed admin/worker config value could overflow the target type.
Add three helpers in weed/plugin/worker/config.go that wrap
ReadInt64Config and clamp out-of-range values back to the caller's
fallback:
ReadInt32Config (math.MinInt32 .. math.MaxInt32)
ReadUint32Config (0 .. math.MaxUint32)
ReadIntConfig (math.MinInt32 .. math.MaxInt32, platform-portable)
Update each flagged call site in the four moved task packages to use
the bounds-checked helper. For protobuf uint32 fields (volume IDs)
the variable type also becomes uint32, removing the trailing
uint32(volumeID) casts and changing the "missing volume_id" check
from `<= 0` to `== 0`.
Touched files:
weed/plugin/worker/config.go
weed/worker/tasks/balance/plugin_handler.go
weed/worker/tasks/erasure_coding/plugin_handler.go
weed/worker/tasks/vacuum/plugin_handler.go
* refactor(worker): use ReadIntConfig for clamped derive-worker-config helpers
CodeQL still flagged three call sites where ReadInt64Config was being
narrowed to int after a value-range clamp (max_concurrent_moves <= 50,
batch_size <= 100, min_server_count >= 2). The clamp is correct but
CodeQL's flow analysis didn't recognize the bound, so it flagged them
as unbounded narrowing.
Switch to ReadIntConfig (already int32-bounded by the helper) for
those three sites, drop the now-redundant int64 intermediate variables.
Also drops the now-unused `> math.MaxInt32` clamp in
ec_balance.deriveECBalanceWorkerConfig (the helper covers it).
|