[feature](compression) Support per-column compression for non-cloud - #66169
[feature](compression) Support per-column compression for non-cloud#66169zwy991114 wants to merge 8 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
1 similar comment
|
run buildall |
|
/review |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
There was a problem hiding this comment.
Requesting changes for seven P1 findings and one P2 finding. No additional user-provided review focus was supplied, so I reviewed the whole PR.
Critical checkpoint conclusions:
- Goal and scope: the PR coherently adds a per-column codec and optional level across SQL, FE catalog/protocol, tablet schema, and both segment writers. The ordinary non-cloud scalar path is largely connected, but cloud/full schema change, compression-only MODIFY, light CCR SQL, complex AGG_STATE storage, codec lifetime/accounting, and test gaps mean the goal is not safely complete.
- Size and focus: the cross-module scope is necessary for this feature and unrelated production changes were not found.
- Concurrency and lifecycle: codec pools are mutex-protected, writer ownership keeps the observer pointer valid, and no new race, lock-order, or deadlock issue was found. The new owned-codec lifecycle does create the retained-workspace multiplier and allocation/free tracker mismatch called out inline.
- Configuration and compatibility: no configuration item is added. Optional thrift/protobuf/footer fields remain readable by compatible readers; old BEs can ignore the write policy during a rolling upgrade and emit the table codec, which is storage-policy drift rather than unreadable data.
- Parallel paths and conditions: legacy, vertical, row-binlog, schema-change, and compaction writers implement small-segment suppression > explicit override > table default when every physical schema node carries the policy. The cloud protobuf omission and complex AGG_STATE child/auxiliary metas are the exceptions. Compression in general Column.equals() also reaches routing/type guards that should not treat it as a logical change.
- Tests and results: none of the changed tests observes requested-level application; the regression write is below the compression-suppression threshold, and the suite lacks the required generated result contract. Per the review-only task contract I did not run local builds/tests; this checkout also lacks .worktree_initialized, thirdparty/installed, and protoc. Live CI currently has compile, FE UT, macOS BE UT, style, license, and secret checks passing; BE UT is ERROR, Cloud UT is FAILURE, several regression/performance checks are pending, and the PR title checker is failing.
- Observability and persistence: SHOW CREATE from the catalog column renders the policy, but reconstructed command SQL omits it. Local edit-log replay retains schema objects; the light CCR binlog executes the incomplete rawSql and loses the clause. Full cloud shadow-tablet protobuf construction separately drops the fields.
- Data writes and transactions: no visible-version, transaction, delete-bitmap, or atomicity mechanism is changed. Segments remain readable because the actual codec is stored in the footer, but affected paths silently write a different policy from the DDL.
- Performance: a private reusable native workspace per levelled column/open segment is an unbounded schema-width multiplier and is not included in segment buffer estimation.
- Other: no security-sensitive behavior, new metric, or dynamic configuration requirement was introduced.
PR metadata also needs cleanup: the title does not match the required bracketed format (for example, [feature](compression) Support per-column compression for non-cloud); Issue Number and Related PR still contain #xxx; and this user-visible feature is marked both Release note: None and Behavior changed: No while documentation is checked without a link.
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 29478 ms |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-DS: Total hot run time: 177512 ms |
ClickBench: Total hot run time: 25.77 s |
99ea75a to
3c2c514
Compare
|
run buildall |
|
/review |
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29696 ms |
TPC-DS: Total hot run time: 177250 ms |
ClickHouse当前PR最接近ClickHouse 的设计:压缩配置属于列定义,并且算法参数使用函数式语法。 它还支持组合预处理 Codec,例如 Delta 后接 ZSTD,并支持通过 ADD COLUMN、MODIFY COLUMN 修改 Codec。(clickhouse.com (https://clickhouse.com/docs/get-started/sample-datasets/wikistat)) Amazon RedshiftRedshift 使用列级 ENCODE: 也可以让系统自动选择: Redshift 的 encoding 是算法或数据编码类型,不暴露 ZSTD 数字等级。(docs.aws.amazon.com (https://docs.aws.amazon.com/redshift/latest/dg/r_CREATE_TABLE_NEW.html)) StarRocksStarRocks 目前是表级属性: 支持 LZ4、ZSTD、zlib 和 Snappy;只能建表时设置,不能按列或通过 ALTER 修改,也不暴露压缩等级。(docs.starrocks.io (https://docs.starrocks.io/docs/table_design/data_compression/)) DuckDBDuckDB 的原生表通常不在列定义中指定压缩算法,显式算法主要用于 Parquet 输出: 这是文件级配置,不属于表的持久化 Schema。(duckdb.org (https://duckdb.org/docs/current/data/parquet/overview)) |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29089 ms |
TPC-DS: Total hot run time: 166789 ms |
ClickBench: Total hot run time: 25.08 s |
|
run buildall |
1 similar comment
|
run buildall |
|
/review |
There was a problem hiding this comment.
Requesting changes for three new P1 findings. Two normal full-review passes plus a separate risk-focused pass converged after Round 2 with NO_NEW_VALUABLE_FINDINGS; existing live threads and prior review summaries were duplicate-suppressed. No additional user-provided focus was supplied, so the whole 29-file PR was reviewed.
Critical checkpoint conclusions:
- Goal and proof: the PR largely connects per-column ZSTD/LZ4HC policy from DDL through FE catalog/protocol metadata, TabletSchema, both segment writers, and level-aware codecs. The goal is not complete because SQL materialized-view projection drops the policy, a rejected compound ALTER can partially mutate live metadata, and the replacement codec registry can retain aggregate historical concurrency peaks. Tests prove parsing, metadata round trips, both writer metadata builders, and actual first/reused-context level effects with compressed-byte comparisons; the three reported scenarios are not covered.
- Size, clarity, and focus: the cross-module surface is necessary and no unrelated production change was found. Current serializers and copy-based paths are straightforward; the fresh SQL-MV constructor and clause-local ALTER rejection are the two missed parallel/control-flow paths.
- Concurrency: concurrent segment/page compression shares the new registry. Registry mutation is mutex-protected, codec construction stays outside the registry lock with insertion reconciled under lock, and per-codec context pools have their own locks; no race, lock-order, or deadlock defect was found. The remaining concurrency-related problem is retained capacity, reported inline.
- Lifecycle: registry-owned codec pointers remain stable across map rehash and production instances intentionally live for the process. Context reset/error destruction and native allocation/free tracker symmetry are sound. That lifetime nevertheless makes every used level's unbounded idle-context peak permanent, which is the reported lifecycle/performance defect; no separate cross-TU static-order issue was found.
- Configuration: no configuration item is added, so dynamic-config propagation does not apply. Explicit levels are FE-bounded and absent/non-positive levels correctly select existing codec defaults.
- Compatibility: the optional Thrift/protobuf/footer fields preserve byte readability because decompression needs the stored codec, not the level, and enum identities are compile-time guarded. The previously raised old-BE rolling-upgrade policy-loss concern remains valid and was not duplicated here; current new-BE local/cloud producers carry both fields.
- Parallel paths and conditions: SegmentWriter, VerticalSegmentWriter, inherited row-binlog, load, partial update, schema change, segment compaction, vertical compaction, and cloud rowset output reach the updated metadata builders for supported scalar/nullable columns. Complex, VARIANT, and AGG_STATE overrides are rejected. DEFAULT parsing, new non-reserved codec keywords, type/range checks, and SQL reconstruction are sound. SQL materialized views and compound-ALTER preflight are the reported exceptions.
- Tests and results: changed FE/BE tests use deterministic assertions, the regression suite follows drop-before-use, ordered-result, expected-error, and generated-output form, and BE tests include real compressed-byte level oracles. Missing SQL-MV, compound-ALTER atomicity, and aggregate multi-level retention tests are requested inline. This was a static review; builds and tests were not run under the review-task contract.
- Observability: no new distributed state machine or user-tunable runtime service is introduced. Existing stored footer metadata and status/error surfaces are sufficient once the policy is propagated correctly; no new metric or log requirement was identified.
- Persistence, transactions, and data writes: catalog Gson/edit-log/image state, local ColumnPB, cloud ColumnPB, rowset TabletSchema, and segment footer persistence were traced. Visible-version, commit, MoW delete-bitmap, and crash-readability behavior are unchanged. The compound-ALTER finding is a failure-atomicity violation because live bloom-filter metadata changes without a job or edit log.
- FE/BE variables: codec and level are passed through current
TColumn,ColumnPB,TabletColumn, andColumnMetaPBpaths in both deployment modes. The SQL-MV issue loses them before those producers, rather than inside the wire conversion. - Performance: sharing by
(codec, level)removes the former per-column workspace multiplier, but the new process-lifetime registry has no aggregate idle-context/byte bound, so sequential multi-level peaks accumulate. No other material CPU, allocation, or repeated-scan regression was found. - Other: no additional correctness, nullable-layout, security-boundary, configuration, transaction-visibility, documentation-path, or observability issue survived the final sweep. The linked documentation PR covers this user-visible feature; PR description syntax/scope metadata should be kept aligned with the final implementation.
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29745 ms |
TPC-DS: Total hot run time: 159106 ms |
ClickBench: Total hot run time: 23.87 s |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29704 ms |
TPC-DS: Total hot run time: 159637 ms |
ClickBench: Total hot run time: 23.86 s |
FE Regression Coverage ReportIncrement line coverage |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Doris only supports a single compression codec configured at the table level (via PROPERTIES("compression"=...)), applied uniformly to every column. In real workloads, columns have very different data characteristics — highly redundant text columns (URLs, referers, search phrases) compress far better under a high-level ZSTD/LZ4HC codec, while numeric/low-cardinality columns gain little and pay the CPU cost. There was no way to tune the codec per column.
This PR adds per-column generic compression codec support for non-cloud (OLAP) tables. Users can specify a codec and optional level directly on a column:
Compression ratio test on ClickBench hits:
Loaded the ClickBench hits dataset into two tables with identical schema. One table (baseline) uses the ZSTD level 3 default on every column; the other overrides five heavy text columns with COMPRESSION 'zstd:19'. After forcing full compaction on both, per-column sizes were read from information_schema.column_data_sizes.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)