[Experiment] Antalya 26.6: Parquet v3 constant column skip - #2181
Open
UnamedRus wants to merge 51 commits into
Open
[Experiment] Antalya 26.6: Parquet v3 constant column skip#2181UnamedRus wants to merge 51 commits into
UnamedRus wants to merge 51 commits into
Conversation
When a Parquet column chunk provably holds a single value in every row - its min/max statistics have `min_value == max_value`, no nulls, and the value is exact - the reader no longer fetches or decodes that chunk's data pages. Instead `detectConstantColumn` records the value and `decodePrimitiveColumn`/`formOutputColumn` materialize it directly. This skips the offset index, column index, dictionary page and data page reads for such chunks (the row group already passed the key condition via its `min == max` hyperrectangle), which is a byte-level I/O win for wide constant columns, plus the decode/decompression CPU. Restricted to flat, top-level primitive columns with no element nulls. For `BYTE_ARRAY`/`FIXED_LEN_BYTE_ARRAY` the writer may truncate min/max, so `min == max` is trusted only when `is_min_value_exact` and `is_max_value_exact` are both set; fixed-width numeric types are never truncated. The value is taken from `PageDecoderInfo::decodeField`, which yields it in the final output (post-cast) domain - e.g. `DateTime` written as `TIMESTAMP_MILLIS` decodes to seconds, not the raw millisecond `decoded_type`. So the constant is materialized directly in the output type, bypassing the `decoded_type` column and `castColumn`. Gated by the new setting `input_format_parquet_use_constant_column_optimization` (default on). A new `ParquetConstantColumnChunks` ProfileEvent counts materialized chunks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two independent scheduling fixes for the v3 reader on large remote files. A. Split the per-stage memory budget from the thread budget. Previously `Stage::memory_target_fraction` drove both a stage's memory watermark and its thread count (`getLimitsPerReader`), and every active stage defaulted to an equal 0.2 share. So `ColumnData` - the one stage that decodes large row groups - was capped at 0.2 of both memory and threads, and on a single large cross-region file only ~2 row groups were read/decoded ahead, leaving the link idle. `Stage` now has a separate `thread_target_fraction`; `getLimitsPerReader` takes both. `ColumnData` gets the lion's share of memory and a larger thread share, while the small, latency-bound index/bloom reads keep enough threads for parallel small reads. The split is static and wants a perf run to tune; it still couples prefetch depth to decode concurrency (decoupling those is a separate, larger change). B. Charge decoded output by its actual footprint. The memory reserved for a subchunk before decoding was an estimate (`estimateColumnMemoryBytesPerRow`) that undershoots for long strings and skewed data, so real RAM overshot the watermark and the overshoot grew with decode-ahead depth. After `decodePrimitiveColumn`, reconcile the `MemoryUsageToken` up to the real `allocatedBytes` of the decoded column, offsets and null maps (grow-only, fail-closed), so the stage counter is honest and the scheduler stops decoding ahead before RAM exceeds the cap. Both are internal scheduling/accounting changes with no query-result change. NOT YET BUILT OR PERF-TESTED; the stage split numbers are a starting point. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the placement of the honest-accounting reconciliation from the previous commit. It ran in `runTask` after `decodePrimitiveColumn` returned, but for the common single-primitive column the function's tail already `std::move`s `subchunk.column` into the output via `formOutputColumn`, so the measurement saw a null column and was a no-op. Thread `MemoryUsageDiff &` into `decodePrimitiveColumn` and reconcile the `MemoryUsageToken` up to the real `allocatedBytes` of `subchunk.column` (plus array offsets and the group null map) at the end of decoding, just before the bookkeeping that may move the column out. Grow-only, fail-closed. Matches the earlier `parquet/honest-memory-cap` prototype. Still not built or perf-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two gate bugs made detectConstantColumn reject every column, so the optimization never triggered on real files. Found by building and testing against CH-written Parquet. 1. Null gate. The CH writer omits `null_count` for physically non-nullable (`REQUIRED`) columns, but the gate required `null_count` to be present and zero, rejecting every non-nullable column. A `REQUIRED` column can have no nulls regardless of the statistic, so only require zero `null_count` when the column is physically nullable (max definition level > 0). 2. Structural gate. `levels[0]` is a synthetic root sentinel with `is_array = true`, so a flat `REQUIRED` column's `levels.back()` IS that root (`size() == 1`, `is_array == true`) and a `Nullable` column has `size() == 2`. The old `levels.size() == 1 && !levels.back().is_array` test therefore rejected exactly the flat columns it meant to accept. Replace with the correct flatness signals (`levels.back().rep == 0` and `max_array_def == 0`), keep the `group_nullable` exclusion, and require the output column to be primitive (top-level, not a Tuple/Map/Array leaf) via `output_columns`. Verified on a CH-written file (1M rows, constant Int64/String/DateTime/ Nullable columns): the optimization now fires (`ParquetConstantColumnChunks` = columns x row groups), results are correct (including the TIMESTAMP_MILLIS -> DateTime seconds path), reading the constant columns drops `ParquetPrefetcherReadRandomRead` 10 -> 1 and `ParquetDecodingTasks` 96 -> 64, and a varying column is correctly not treated as constant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tch stage) Adds a ColumnDataPrefetch pipeline stage between OffsetIndex and ColumnData. It runs `determinePagesToPrefetch` and issues the compressed data-page reads (`startPrefetch`) but does not decode; ColumnData then only decodes from the already-in-flight buffers. The two stages have separate memory budgets: ColumnDataPrefetch gets a large share (compressed row groups are cheap, ~tens of MB) so many row groups can have their reads outstanding, while ColumnData gets a bounded share (decoded row groups are large, ~hundreds of MB) that caps how many are decoded/resident at once. Because row groups are independent and their reads run in the Prefetcher's own io pool (not the parsing threads), fetch depth is now decoupled from decode-ahead depth - the fetch-deep / decode-shallow mode that finding #5 called for. Within a row group subgroups stay sequential, so `determinePagesToPrefetch`'s in-order requirement is preserved. The compressed-read memory is charged to the ColumnDataPrefetch stage via startPrefetch and released when ColumnData resets the prefetch handles (the handle records its allocating stage, so the release credits the right budget regardless of which stage's diff performs it). Verified on the Debug build: reads are correct with and without PREWHERE (3M-row multi-row-group file), the constant-column optimization still fires, and there is no deadlock. Perf/prefetch-depth gains need a high-RTT remote (S3) run to observe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds read back-pressure on the ColumnDataPrefetch stage: once the compressed data already in flight covers more than `input_format_parquet_prefetch_bandwidth_hide_seconds` of the measured read throughput, stop prefetching further ahead. Beyond that point the storage link is already fed, so extra compressed buffering only wastes memory without improving throughput (the "+21 GB RAM for +3.6 s" case in finding #4). The Prefetcher now tracks completed-read throughput (bytes since init / elapsed, `averageThroughputBytesPerSec`). The scheduler compares it against the ColumnDataPrefetch stage's in-flight compressed bytes (that stage's memory usage) and stops admitting more prefetch tasks when the in-flight bytes exceed throughput x hide_seconds. The privileged-task escape (lowest incomplete row group is always schedulable) still applies, so back-pressure can never deadlock. Defaults to 0 (disabled): the throughput heuristic and the hide-seconds target can only be validated on a high-RTT remote (S3) run, and on local fast IO the estimate is meaningless, so it is opt-in for cluster tuning. With C's ColumnDataPrefetch memory budget already providing a static cap on compressed buffering, this setting makes that cap adaptive. Verified on the Debug build: results are correct with the setting off (default) and with an aggressively small value (0.001s) that forces heavy throttling - no deadlock, identical results. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port adaptation: antalya-26.6 predates the Nullable(Tuple) support on master, so PrimitiveColumnInfo::group_nullable and ColumnSubchunk::group_null_map do not exist there. Remove those references from the constant-column detection gate and the decoded-memory reconciliation. Both are safe: nested-in-nullable-struct leaves are already excluded from the constant-column optimization by the `output_columns[...].is_primitive` guard, and the group null map (when it would exist) is negligible in the memory reconciliation. Also resolved during the rebase onto antalya-26.6: - ProfileEvents/Reader: keep only the new ParquetConstantColumnChunks event (antalya has no ParquetPrunedPages). - Column index uses antalya's singular `column_index_condition`. - Dropped the master-only `pruningMemoryReservation` (antalya's applyBloomAndDictionaryFilters takes no reservation). NOT YET COMPILED against antalya-26.6. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: c6b70b3 I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: 34816a3 I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: f260506 I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: 2a20ab9 I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: 114640e I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: a019cd7 I, UnamedRus <dtitmoav@gmail.com>, hereby add my Signed-off-by to this commit: a3ee936 Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Commit c6b70b3 accidentally added a `unique_key_probe_implementation` entry to `SettingsChangesHistory.cpp` under version 26.8. That setting is not registered anywhere in `Settings.cpp`, so `02324_compatibility_setting` failed with `UNKNOWN_SETTING` when applying `compatibility` to old versions. The entry is unrelated to the parquet constant-column work; remove it. CI: https://github.com/Altinity/ClickHouse/actions/runs/31099889198/job/92610989464 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Extends the constant-column optimization to the all-null case. When a Parquet column chunk provably holds only nulls - its `null_count` statistic equals `num_values` on a physically nullable leaf - the reader no longer fetches or decodes that chunk's dictionary or data pages. `detectConstantColumn` marks the chunk constant (and `is_all_null`), and `formOutputColumn` materializes the result directly: `Null` for a Nullable output, or the output default when `input_format_null_as_default` substitutes nulls for a non-nullable output. A non-nullable output without null substitution cannot represent the result, so such a chunk is left to the normal decode path. Unlike the single-value case this needs no value decode, so it sidesteps the min/max exactness and `BYTE_ARRAY` truncation checks entirely; it only reads the `null_count` count. `formOutputColumn` records every row of an all-null chunk in `block_missing_values` (the single-value case has no nulls and records nothing), matching the normal decode path's null-map bookkeeping so `input_format_null_as_default` stays correct. All-null wide chunks are common in schema-evolved files (a column added later is all-null in older row groups), so this skips dictionary, data page, offset index and column index reads for a frequent real-world shape. Reuses the existing `input_format_parquet_use_constant_column_optimization` setting and the `ParquetConstantColumnChunks` ProfileEvent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Commit c6b70b3 fabricated two entire version blocks (26.8 and 26.7) in SettingsChangesHistory.cpp holding ~80 setting entries pulled in by a bad merge. 63 of them reference settings that are not registered on this branch and 10 duplicate entries recorded elsewhere, so applying `compatibility` to an older version threw `UNKNOWN_SETTING` (e.g. `s3_base`, `unique_key_probe_implementation`), failing 02324_compatibility_setting. The only entry that belongs to this parquet commit is `input_format_parquet_use_constant_column_optimization`. Remove both fabricated blocks and keep just that setting, moved into the pre-existing 26.6 block. The resulting history equals the commits parent plus that single line. Follow-up to 77c2c72 which removed only the first offending entry. CI: https://github.com/Altinity/ClickHouse/actions/runs/31105816361/job/92630866726 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…ds in history The setting was added by a019cd7 but never given a SettingsChangesHistory entry, so 02995_new_settings_history reported it as an undocumented new setting (the failure surfaced once 02324_compatibility_setting stopped failing first). Add it to the 26.6 block next to input_format_parquet_use_constant_column_optimization; default 0 disables the back-pressure and matches the pre-existing behavior. Reproduced the full 02995 check locally (registered settings minus both baseline TSVs minus recent-version history): this was the only missing setting. CI: https://github.com/Altinity/ClickHouse/actions/runs/31110878761/job/92648476492 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The previous run failed in Config Workflow / Set up job with "Failed to resolve action download info. Error: Service Unavailable" - a transient GitHub Actions outage, not a code failure. Empty commit to re-run CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Adds the DataLakeCatalog `namespaces` setting: a comma-separated list of allowed namespaces for `rest`, `glue` and `unity` catalog types, so a DataLake database exposes only the selected namespaces. `rest` supports nested rules (`foo`, `foo.bar`, `foo.*`) via `RestCatalog::AllowedNamespaces`; `glue`/`unity` use a flat allow-set. Default `*` allows everything (pre-existing behavior). Ported from the merged antalya-25.8 PR onto antalya-26.6. Cross-version adaptations: - Catalog construction moved into `DatabaseDataLake` in 26.6, so the namespaces are threaded through `CatalogSettings` / the catalog constructors there; the 25.8 inline construction in `DataLakeConfiguration::getCatalog` (and its `catalog_namespaces` plumbing) is obsolete and dropped, leaving `DataLakeConfiguration.h` unchanged. - `CATALOG_NAMESPACE_DISABLED` error code renumbered 757 -> 779 (757..778 are already taken on this branch). - The namespace filter checks were merged into 26.6-refactored code paths (threadpool-based `RestCatalog::getTables`, extracted Glue credentials provider, 26.6 `resolveMetadataPathFromTableLocation`). - The delegating 6-arg `RestCatalog` constructor (used by OneLake/BigLake, which do not support the filter) defaults `allowed_namespaces` to `*`. - Added explicit `<boost/algorithm/string.hpp>` and `<unordered_map>`/ `<unordered_set>` includes (the upstream PR relied on transitive includes). PR: #1337 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The ported AllowedNamespaces block opened a public: section for the nested class and closed with private:, which downgraded every RestCatalog member after it (loadConfig, getAuthHeaders, retrieveAccessToken, ...) from protected to private. Subclasses (OneLake/BigLake/Paimon REST catalogs) call those, so Build failed with "is a private member of DataLake::RestCatalog". Restore the trailing access specifier to protected; AllowedNamespaces stays public (the gtest references it). CI: https://github.com/Altinity/ClickHouse/actions/runs/31171012685/job/92846936566 PR: #2181 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Adding the namespaces_ parameter to the primary RestCatalog constructor broke the pre-existing unit test gtest_rest_catalog.cpp, which constructed a RestCatalog without it (no matching constructor). Pass namespaces = "*" (allow all) in the new argument slot, before the context argument. CI: https://github.com/Altinity/ClickHouse/actions/runs/31176356154/job/92863225108 PR: #2181 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
formOutputColumn previously expanded a constant column chunk with insertMany(Field, num_rows) - an O(rows) per-row Field-dispatch fill that showed up as a consistent ~20% UserTime increase in profiling (the baseline decoded the same near-constant chunk cheaply via RLE/dictionary). Emit a ColumnConst instead: O(1) to build, and the const-ness propagates through the pipeline. A PREWHERE/WHERE predicate computes its result from the value without expanding the stored column, and GROUP BY / aggregation over the column get a const key. The value is already in the output (post-cast) domain. The all-null block_missing_values bookkeeping is unchanged. The reader path preserves the const: getOrFormOutputColumn returns it as-is, ColumnConst::size() == rows_pass satisfies the delivery checks, and ColumnConst::filter keeps it const through multistage PREWHERE. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Extends the constant-column optimization from whole-chunk (tier 1, footer statistics) to per-row-subgroup granularity using the Column Index per-page min/max/null_pages, which the reader already loads for predicate push-down. Catches columns constant over a run of pages without the whole row group being constant (common in sorted/clustered data), with no change to subgroup/chunk sizing. - constColumnMaterializationEligible: shared flat-top-level-primitive gate, factored out of detectConstantColumn. - applyColumnIndex: retain per-page constant info (value / is_const / all_null) in ColumnChunk::page_const_info. Fixed-width numeric/date/time only for value constants (the Column Index has no per-page exactness flag, so a truncated BYTE_ARRAY min==max is untrustworthy); all_null is a plain flag, always safe. - detectConstantSubchunk: a column is constant for a subgroup iff every page overlapping the subgroup row range is all-null, or all hold the same value. - Wired into intersectColumnIndexResultsAndInitSubgroups; sets the subchunk constant fields so decodePrimitiveColumn skips decode and formOutputColumn materializes a ColumnConst. New ParquetConstantColumnSubchunks ProfileEvent. Tier 1 stays the always-on baseline and the only detector for BYTE_ARRAY. Tier 2 is opportunistic: only where the Column Index is already loaded (predicate push-down columns), never force-fetched. Deferred (follow-up): skipping the prefetch of constant subgroups pages (the I/O win). Currently those pages are still fetched and skipped forward by the next subgroups skipToRowOrNextPage; correct but does not yet save the read. Design: docs/design/parquet-v3-page-level-constant-column.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Completes tier-2: a per-subgroup constant column (detectConstantSubchunk) now also skips fetching its data pages, not just decoding them. In determinePagesToPrefetch, a constant subchunk claims no pages; a page fully inside it is released (never fetched), while a page shared with a neighbouring non-constant subgroup is left for that subgroup to claim. Safe because tier 2 only exists when the Column Index (hence Offset Index) is loaded: a constant subgroup never calls skipToRowOrNextPage, and a non-constant subgroup jumps directly via the offset index to its own claimed pages, so a released constant page between them is never accessed. The whole-chunk data_pages_prefetch is still split (likely_to_be_used=false), so only claimed pages are actually read. This turns the tier-2 CPU/memory win into an I/O win as well (fewer S3 GETs / ParquetPrefetcherReadRandomRead), matching tier 1 but at page granularity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Adds input_format_parquet_use_column_index_for_constant_columns (default off). By default tier-2 per-subgroup constant detection only runs where the Column Index is already loaded (columns with a predicate). This setting force-loads the Column Index (and Offset Index) for eligible read columns without a predicate, so tier-2 can skip single-valued page runs on them too - worthwhile for sorted / low-cardinality columns, at the cost of a small extra (tail-contiguous, coalesced) index read. A force-loaded column takes the same load path as a predicate column (use_column_index = true); applyColumnIndex records per-page constant info but skips page-level predicate pruning when the column has no condition (prev_row_idx stays 0 -> whole chunk selected -> no restriction). Setting is declared in FormatFactorySettings, plumbed through FormatSettings / FormatFactory, and recorded in SettingsChangesHistory (26.6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Collaborator
Author
|
RowGroup level + push ColumnConst to transforms |
…off] Approach B: when only PART of a row subgroup is single-valued (a constant run shorter than, or straddling, a subgroup), fill those pages from the per-page Column Index while decoding only the varying pages - producing a full column where whole-subgroup tier-2 cannot make a ColumnConst. Gated behind input_format_parquet_fill_constant_pages (default off). Additional gates keep it correct and simple: - output column must not need a post-decode cast (we fill the decoded_type column with the Column Index value, valid only when decoded == output value type); - no predicate on the column (so no page pruning; data_pages == all pages); - no prewhere filtering in the subgroup (avoids filter/range intersection); - at least one single-value page and no all-null page in the subgroup (all-null pages fall back to the standard decode). fillConstantPagesAndDecodeRest walks the subgroup page by page: a single-value page is filled via insertMany (+ null-map zeros) without reading it; a varying page is decoded with the existing skipToRowOrNextPage + readRowsInPage, which jumps over the filled pages via the offset index without loading them. First cut is decode-fill only: constant pages are still prefetched (correct, some wasted I/O). Skipping their prefetch needs cross-subgroup coordination and is a follow-up. Design: docs/design/parquet-v3-page-level-constant-column.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Completes the mixed-topology fill (input_format_parquet_fill_constant_pages, default off): a subgroup that fills its single-value pages from the Column Index no longer prefetches them. determinePagesToPrefetch consults willFillConstantPages (the same deterministic predicate the decode path uses) and, for a fill subgroup, does not claim its is_const pages - so a page overlapped only by fill subgroups is released and never read, while a page also decoded normally by another subgroup is still fetched. Also adds a no-prewhere / no-row-level-filter gate to willFillConstantPages so rows_pass == rows_total holds identically at prefetch time and decode time, keeping the two decisions consistent. This turns the mixed-topology fill from a decode-CPU-only win into an I/O win too (fewer page reads), matching tier-2 at sub-subgroup granularity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
min_value == max_value provably means a single exact value for every physical type, including BYTE_ARRAY / FIXED_LEN_BYTE_ARRAY. Statistics and Column Index bounds are always valid (min_value <= every value <= max_value) and truncation only ever widens them, so a truncated value - or any page/chunk with two distinct values - yields min_value < max_value. Equality therefore requires a single value short enough to be stored exactly; the is_*_value_exact flags are implied and, for the Column Index (which has no per-page exact flag), never needed. Remove the BYTE_ARRAY/FIXED_LEN exclusion from tier 1 (detectConstantColumn) and the per-page recording in tier 2 (applyColumnIndex), so constant string columns and string constant page runs get the optimization too. Also fixes tier 1 for writers that omit is_*_value_exact (previously any such BYTE_ARRAY constant was skipped even when short and exact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Previously a mixed-topology subgroup (input_format_parquet_fill_constant_pages) bailed to normal decode if any overlapping page was all-null. Now fill them too: an all-null page appends no non-null values and marks its rows null in the null map; the existing expand() + Nullable-wrap / null_as_default tail finalizes them exactly as the normal decode does (the column holds compact non-null values, the null map covers all rows). determinePagesToPrefetch also skips prefetching all-null pages, matching the fill. willFillConstantPages now accepts all-null pages as fillable, but still bails when an all-null page is present and the output can represent neither null (non-Nullable output) nor a default (no null_as_default) - the standard decode raises the usual not-null error there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Drop the column_index_condition gate from willFillConstantPages. A page-pruning predicate on the column (column_index_condition) is safe for the fill: subgroups are built only over contiguous surviving row ranges, so a pruned page never overlaps a subgroup and the fill never walks it; the prefetch-skip indexes page_const_info by each page global position (page.meta into page_locations), which is correct regardless of the data_pages subset. A column carrying an actual prewhere/row-level filter is still blocked by the no-prewhere gate. Gate 2 (output needs_cast) is intentionally left in place: whether decodeField yields the decoded or the output value domain is ambiguous from static reading (SchemaConverter/convertField vs the tier-1 is_constant branch), and guessing wrong is silent wrong data. It needs a build + a needs_cast test to resolve (and that test would also confirm tier-1/tier-2 on hint-cast columns). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The mixed-topology fill now walks the rows that pass the filter (row_subgroup.filter) instead of the whole subgroup range, so it works under PREWHERE / row-level filters: it iterates passing ranges (like the standard row-range decode) and, within each, fills constant / all-null pages for their overlapping passing rows and decodes varying pages for the contiguous passing sub-range. It therefore produces exactly rows_pass values for any filter, and willFillConstantPages no longer depends on rows_pass - so the decision is identical at prefetch time and decode time and the prefetch-skip can never drop a page the decode needs. Removes the no-prewhere and rows_pass == rows_total gates from willFillConstantPages. The only remaining gate is needs_cast (build-gated; see the decodeField value-domain question). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…prefetch `createReadBuffer` eagerly prefetches the head of "small" objects (up to `2 * max_download_buffer_size`). That heuristic was added for many-small-files streaming formats (CSV/JSON), where reading starts at the beginning of the file, so a from-start read-ahead is exactly what will be consumed. Column-oriented random-access formats (Parquet/ORC/Arrow) instead read the `FileMetaData` footer at the *tail* first and drive their own prefetcher. For an object larger than one download buffer the from-start prefetch cannot cover the footer, so it is dropped on the first positioned read - a wasted object read (`S3GetObject`) that transfers bytes nobody consumes. Gate the eager prefetch on the format's access pattern: for random-access input formats only prefetch when the whole object fits a single download buffer (then it degenerates to one whole-file read the format serves entirely from memory, footer included); above that, leave reading to the format's tail-first prefetcher. Streaming formats keep the previous `2x` threshold, so the original small-files optimization is unchanged. `FormatFactory::checkIfFormatIsRandomAccessInput` reports whether a format is registered via `registerRandomAccessInputFormat[WithMetadata]`. The `createReadBuffer` flag defaults to false, so metadata / manifest / delete-file reads keep their current behaviour. Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The `IcebergMetadataFilesCache` for table metadata is keyed on `(table_uuid, metadata_file_path)`, but the very first read of a table's metadata - the bootstrap in `initializePersistentTableComponents` - is issued with no `table_uuid`, because the Iceberg `table-uuid` is only learned by parsing that very file. `getMetadataJSONObject` therefore takes the uncached branch and stores nothing, so the next read (query state, now with the uuid) misses the cache and re-reads the same metadata file - two reads of one small file per cold table open. Seed the cache from the bootstrap read itself: after parsing the metadata, extract its `table-uuid` and store the already-read JSON under `getKey(table_uuid, path)`, so subsequent reads of the same file hit. No extra I/O - the buffered string is reused via `getOrSet`'s load function, which only runs when the entry is absent. This is safe and preserves the table-recreate guard: the entry is keyed by the uuid parsed from this exact file, and an immutable metadata file always yields the same `(uuid, path)`; a dropped-and-recreated table has a different `table-uuid` and thus a different key. Only the top-level table metadata carries `table-uuid`; other JSON is left untouched. Signed-off-by: UnamedRus <dtitmoav@gmail.com>
A data-lake catalog (Iceberg REST, Glue, ...) already parses the Iceberg `table-uuid` out of its `LoadTable` response (`RestCatalog::setTableUUID`), but it was only used to build the ClickHouse `StorageID` - it never reached the Iceberg metadata layer. So the bootstrap metadata read still ran with no uuid and could not key the metadata files cache, forcing a redundant re-read of the same `metadata.json` on the following query-state read. Forward the catalog's `table-uuid` as the `iceberg_metadata_table_uuid` storage setting (`DatabaseDataLake`), and have `initializePersistentTableComponents` read it and pass it to `getLatestOrExplicitMetadataFileAndVersion` and `getMetadataJSONObject` instead of `std::nullopt`. The bootstrap read then keys the cache on `(table_uuid, path)` on the first read, and the later read hits. Resolution is unchanged: the catalog also sets `iceberg_metadata_file_path`, so `getLatestOrExplicitMetadataFileAndVersion` still takes the explicit-path branch (the uuid argument is inert there). For access paths with no catalog (bare-path `iceberg()` table function, Hadoop / version-hint tables) the setting stays unset and the uuid is still learned from the file - handled by the cache seeding in `getMetadataJSONObject`. Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…e HEAD
Opening an object went through a HEAD (getObjectMetadata) on every read to learn
its size and ETag, and the result was not cached process-wide, so a scan
re-HEADed every candidate file on every query (traced: ~1 S3 HeadObject per
candidate file, e.g. ~1900 HEADs to read ~550 files). The HEAD is also
mandatory-before-cache-lookup, because the filesystem / page / parquet-metadata
caches key on (path, etag) and the etag comes from the HEAD.
Introduce a process-wide identity cache and a single-request metadata fetch:
* `S3::getObjectIdentity` issues one `GetObjectAttributes` request (size + ETag +
multipart part sizes) and falls back to a plain HEAD if the API is
unsupported, denied, or errors - safe against S3-compatible stores. The ETag
is quote-normalized so the GetObjectAttributes and HEAD paths yield the same
identity (they are used as cache keys). Adds the `GetObjectAttributes` wrapper
to `S3::Client` (mirrors `GetObjectTagging`) and the request alias.
* `ObjectStorageIdentityCache` (experimental singleton) maps path ->
{etag, size, part_offsets}. `S3ObjectStorage::getObjectMetadata` consults it on
the identity path (no tags requested): a hit skips the request entirely; a miss
fetches via `getObjectIdentity` and populates it. The tags path and the
credentials-refresh retry are unchanged.
* `ObjectMetadata` gains `part_offsets` (cumulative multipart part offsets),
derived from the per-part sizes; consumers use it to align reads.
* `IcebergDataObjectInfo` no longer pre-populates object metadata from the
manifest, so Iceberg data files also go through this single identity path and
get the real ETag and multipart layout instead of a synthesized identity.
New ProfileEvents: `S3GetObjectAttributes`, `ObjectStorageIdentityCacheHits`,
`ObjectStorageIdentityCacheMisses`.
Note: the identity path returns no user-metadata attributes (GetObjectAttributes
does not provide them); callers that need them still use the tags path. The
cache is an experimental singleton (not Context-managed / not SYSTEM DROP-able)
and should be promoted before productionization.
Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Experimental, off by default. When the object's multipart-upload layout is known (from GetObjectAttributes, via the object-storage identity cache), constrain the Prefetcher's range coalescing to a single part window so one read never straddles two parts - an AWS byte-range best practice for multipart-uploaded objects. * `ReadOptions.multipart_part_offsets` carries the part boundaries; populated in `ParquetV3BlockInputFormat` from `ObjectMetadata.part_offsets`, gated by the new setting so it can be A/B-compared. * `Prefetcher` keeps the offsets and, in `pickRangesAndCreateTaskIfNotExists`, stops extending a task past the boundaries of the part containing the initial range. This only constrains coalescing; a single requested range larger than a part is left as is (would require splitting). * New setting `input_format_parquet_align_reads_to_multipart_boundaries` (default false) and ProfileEvent `ParquetPrefetcherPartAlignedTasks` (counts tasks whose coalescing was cut at a part boundary) for measurement. Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…ent guard) Generalizes the experimental multipart read alignment so it can be tuned and used without probing per-file layout. * input_format_parquet_read_alignment_bytes (UInt64, 0=off): align coalesced reads to a fixed byte grid (e.g. the writer's multipart part size - 10 MiB for delta-rs, 64 MiB for Spark/S3A) so no read straddles a multiple of it. Needs no GetObjectAttributes / identity cache, so alignment can be measured in isolation. * input_format_parquet_read_alignment_min_bytes (UInt64, default 1 MiB): anti-fragmentation guard - don't cut a read at a boundary when the aligned segment would be smaller than this; allow the straddle instead of emitting a tiny extra request. The Prefetcher derives the boundary window from the real per-file part offsets when known (input_format_parquet_align_reads_to_multipart_boundaries), else from the fixed stride; the min-bytes guard disables the cut for tiny segments. New ProfileEvent ParquetPrefetcherAlignmentSkippedSmall counts those skips (ParquetPrefetcherPartAlignedTasks already counts constrained tasks). Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…include
Two compile fixes surfaced by a local build:
* The new `ObjectMetadata::part_offsets` field (added with the object-storage
identity cache) broke designated-initializer sites under
`-Werror,-Wmissing-designated-field-initializers`. Add `.part_offsets = {}` to
the `ObjectMetadata{...}` initializers in the S3, HDFS and Azure object
storages.
* `IcebergWrites.cpp` uses `getDecimalScale` (for the Time64 branch) but did not
include `<DataTypes/DataTypesDecimal.h>`, where it is declared - add the
include.
Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Standard S3 GET has a fat tail (p50 ~5 ms vs p99 100+ ms), so a single slow read on the critical path stalls a decode task (visible as large ParquetFetchWaitTimeMicroseconds). Hedging: if a read a consumer is blocked on doesn't finish within a threshold, issue a duplicate read and use whichever returns first (S3 GET is idempotent). Phase A (this commit) runs the hedge synchronously on the already-blocked consumer thread: getRangeData waits up to the threshold via a new CompletionNotification::wait_for, then reads the same range into task->hedge_buf and serves from it, notifying so sharers wake. This has no async-lifetime hazard - the hedge finishes before the PrefetchHandle is released. A fully async race (Phase B) is deferred: it would need decreaseTaskRefcount to defer freeing hedge_buf until an in-flight hedge finishes (the refcount/Deallocated path assumes a single reader), which risks a use-after-free if done naively. Scope: remote (RandomRead) reads only, no larger than a size cap (latency, not throughput), one hedge per task, bounded by a concurrency cap (cost control). Settings (all experimental, off by default): * input_format_parquet_hedged_read_threshold_ms (0 = off) * input_format_parquet_hedged_read_max_bytes (default 4 MiB) * input_format_parquet_hedged_read_max_inflight (default 4) ProfileEvents: ParquetPrefetcherHedgedReads, ParquetPrefetcherHedgedWins. Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…eBase with Context management Two object-storage read-path improvements that share the server-settings and startup wiring, so they land together. Latency-aware host selection (opt-in, off by default): - `HostResolver` keeps a per-address EWMA of observed request time-to-first-byte, fed back from the S3 client (`PocoHTTPClient`). `selectBest` biases the weighted-random address choice towards lower-latency front-ends, the connection pool hands out the fastest pooled connection at dispatch, and opens fresh connections to fast front-ends instead of reusing slow-backend ones. Server setting `http_latency_aware_host_selection` (applied in both server and `clickhouse-local`). Object-storage identity cache productionization: - Replace the hand-rolled clear-on-overflow map in `ObjectStorageIdentityCache` with `CacheBase` (SLRU, size-bounded, per-entry weight, `CurrentMetrics` for bytes/cells). - Make it Context-managed: server settings `object_storage_identity_cache_size` / `_policy` / `_size_ratio`, reachable from the low-level object-storage layer via `Context::getGlobalContextInstance` (null-safe fallback to an uncached request), and `SYSTEM DROP OBJECT STORAGE IDENTITY CACHE`. Verified end-to-end on same-region AWS S3: warm reads issue zero `GetObjectAttributes`, `SYSTEM DROP OBJECT STORAGE IDENTITY CACHE` resets the cache, and a size of 0 disables it. Latency-aware selection is neutral on a healthy same-region path (concurrency hides per-request latency) and is intended as opt-in insurance for high-latency / degraded-front-end conditions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…plification Read coalescing bridges gaps shorter than the seek threshold to save requests, but many sub-threshold gaps accumulate into a task that is almost entirely filler: reading a tiny, scattered (RLE / near-constant) column drags in its neighbouring columns and amplifies a few KB of wanted data into hundreds of MB read. Add `input_format_parquet_read_min_fill_ratio` (0 = off): during coalescing, `Prefetcher` stops extending a task once bridging the next gap would drop its wanted/span fraction below the ratio. Gaps below a small absolute floor are always bridged, so dense reads (fill ~1) are never split. This is the inner segmentation step of a top-down plan whose outer partition is the existing part-boundary guard (`gapCrossesBoundary`), so it composes with alignment and the read splitter without re-merging. New profile event `ParquetPrefetcherFillRatioLimitedTasks`. Measured on a same-region AWS S3 Iceberg table (100M rows): a scan of one sparse column drops from ~209 MB to ~43 KB read and ~2.4x faster wall; a genuinely non-constant sparse column benefits equally (~211 MB to ~50 KB), which page-level constant skip could not do. Dense scans are unchanged (guard never fires). A mixed keys+metric scan is unchanged (the metric's dense tasks keep fill high), with no regression. Also strips temporary footer debug logging and now-unused `logger_useful.h` includes, and lands the accompanying in-progress v3 read-path work on this branch (parallel segment reads / speculative footer plumbing, Iceberg footer-size hint via split offsets). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
… IDENTITY CACHE The new `SYSTEM DROP OBJECT STORAGE IDENTITY CACHE` privilege (added with the Context-managed identity cache) adds a row to `SHOW PRIVILEGES`, so the stateless test reference needs the corresponding line. CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2181&sha=5b16210f9e7dbf582ab7e23983337a9244f2b9fe&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Building the list of Iceberg data files to read was single-threaded: one producer thread drained `SingleThreadIcebergKeysIterator`, which for every manifest entry deserializes the per-column lower/upper bounds and evaluates the min/max `KeyCondition` to prune. On a wide table (e.g. ~17.5k data files, of which ~17.5k are pruned to a few dozen survivors) this serial pass is the dominant startup stall: reader threads block in `IcebergIterator::next` on the `blocking_queue` until the producer reaches the surviving entries, and the `IcebergMetadataFilesCache` only caches the manifest file contents, not the per-entry pruning, so the work is repeated every query. The manifest-processing stack was already built for concurrency — `ManifestFileIterator::next` uses an atomic row cursor, the Avro deserializer guards its per-row cache with a `SharedMutex`, and the pruner/schema caches are mutex- or `SharedMutex`-guarded — but nothing drove it in parallel. Add `iceberg_metadata_processing_threads` (default 1 = the previous serial producer). When greater than 1 (0 = auto = number of CPU cores), spawn that many producer threads that share the manifest-file cursor through a small mutex (only the cheap per-file advance — `getManifestFile` + build — is serialized) and drain each manifest file's entries concurrently, so the heavy per-entry bound-deserialize and min/max pruning is spread across threads. The last worker to finish closes the queue; exceptions propagate through the existing `exception`/`exception_mutex` path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Collaborator
Author
S3 frontend->backend latencyTLDR: Some of them are slower/some of them are faster. For relatively huge GET requests (over dozens of MB), prefetches, (or requests at later stage of query, when there is already enough data in fly), it probably wouldn't have huge affect.
It probably could add extra 60ms to query execution (ie sum of extra sequential latency, wait itself in total will be bigger probably) if we unlucky enough. Basically, idea was to store TTFB alongside statistics of certain connection, sort and return next free connection by that/open new connection to fast frontend instead of reusing one for slower. |
Collaborator
Author
Straddle across MultiPart boundariesWhen object is created using multipart upload, AWS S3 and some other backends doesn't "glue" parts together, they are likely are not being moved from where they were written. There are multiple sources, which decide byte range associated with certain read request:
Probably, seek and prefetch could be "optimized" not to glue or split requests at boundaries between parts. |
Collaborator
Author
Object identity cacheClickHouse have multiple caches for Parquet/Iceberg/Data, but some of them are cached on: This does make sense for certain use cases with s3, (not to use expired cache if object is changed)
But, this protection (against expired object) doesn't make sense for iceberg tables model, because in their consistency model, they do not store etag value in manifest lists. So, even if object is replaced we will not know/detect that. So, idea was to introduce very small cache just for those engines, which does not fix in contract value of etag. And use it, in order to probe those caches, which use object_path + etag as key. |
Collaborator
Author
Parquet footerWhen reading Parquet file:
So, total latency was (GET 1) + (GET 2) So, we either can be bit more generous and just read more data for GET 1 request (in hope that whole footer will land in it) Another option, in case of big estimates, do 2 requests in parallel:
|
Collaborator
Author
Guard for *_min_bytes_for_seekClickHouse glue reads in one GET request, if there is less than 4MB gap between them Which in general, was tradeoff we were willing to accept, but as always there should be some limit to that. So, idea was to allow creation of more GET requests, for some pathological cases. |
Collaborator
Author
Hedged requestsSend duplicate GET request, if original is not answering (TTFB? total latency?) for longer than X Parallelize iceberg manifest calculationDone in #1753 |
Collaborator
Author
Constant Column optimizationIcebergIceberg store metadata/statistics for some columns.
Done in #1069 ParquetParquet in footer themself store statistics for all columns in file for RG. is safe to use., then again we can avoid read this column and push it as const in chunk. Another case, For non numerics, if min_values/max_values are truncated, max_value should be truncated-and-increment. And again, we can do the same optimization (not read page/range of pages), create const column in chunk |
ianton-ru
reviewed
Aug 11, 2026
ianton-ru
left a comment
ianton-ru
left a comment
There was a problem hiding this comment.
I count at least 5 different independent features in this PR
- Cache for S3 object metadata
- Host resolving with weights based on latency
- Hint for parquet footer size
- Parallel parquet file loading
- Constant columns
Plus namespace filter from #1019
This is not PR for merging, only as a proof of concept.
| auto fetch = [&](const S3::Client & c) -> S3::ObjectInfo | ||
| { | ||
| if (identity_only) | ||
| return S3::getObjectIdentity(c, uri.bucket, path, /*version_id=*/ {}); |
There was a problem hiding this comment.
Does this make something faster?
getObjectInfo makes HeadObject request, getObjectIdentity makes GetObjectAttributes with fallback to getObjectInfo.
As I understand, GetObjectAttributes give profit if replaced several different requests, but in current case it replaces only one.
Fallback makes only worse for some old S3-compatible storages without GetObjectAttributes support.
Collaborator
Author
There was a problem hiding this comment.
Does this make something faster?
No, it's not faster.
It was attempt to gather/cache real multiPart boundaries from S3 via GetObjectAttributes.
But, it need special conditions (multiPart upload with extra checksums) AND have access to GetObjectAttributes.
So, probably if it will be implemented using Head is more reliable.
| if (auto global_context = Context::getGlobalContextInstance()) | ||
| identity_cache = global_context->getObjectStorageIdentityCache(); | ||
|
|
||
| if (identity_cache) |
There was a problem hiding this comment.
As I understand this caches also metadata during direct reading with S3 table function. Cache is good for immutable objects, "owned" by ClickHouse, but not for all objects on S3.
Collaborator
Author
There was a problem hiding this comment.
As I understand this caches also metadata during direct reading with S3 table function.
It might, and if so, need to be fixed, but doesn't affect bench for now.
Cache is good for immutable objects, "owned" by ClickHouse, but not for all objects on S3.
I think, we should do it for "any" immutable kind of object. (either ClickHouse owned or Iceberg catalog).
We potentially could do it for any object of any source, if we use conditional GET later (GET object/range if etag match X, where X is cached value, so we can get cache invalidation & data request in one go, but it's unnecessary for Iceberg)
| size_t num_columns = std::max(entry.columns_infos.size(), entry.value_bounds.size()); | ||
| if (num_columns > 0) | ||
| { | ||
| constexpr size_t rows_per_row_group_guess = 1'000'000; |
There was a problem hiding this comment.
Does this number have grounding? From my point of view, guessing "set initial footer size based on random constant number of rows per group" is not worse and not better than "set initial footer size as 64 kB"
Collaborator
Author
There was a problem hiding this comment.
is not worse and not better than "set initial footer size as 64 kB"
Requesting larger footer, up to 2MB is basically free.
And if we can save 1 GET request almost for free...
┌────────┬─────┬────────────┬─────────────────┬──────────────┐
│ footer │ RTs │ oracle p50 │ spec_serial p50 │ spec_par p50 │
├────────┼─────┼────────────┼─────────────────┼──────────────┤
│ 256 K │ 1 │ 26.7 │ 33.6 │ 34.3 │
├────────┼─────┼────────────┼─────────────────┼──────────────┤
│ 1 MB │ 1 │ 30.1 │ 33.9 │ 33.1 │
├────────┼─────┼────────────┼─────────────────┼──────────────┤
│ 2 MB │ 1 │ 33.7 │ 33.0 │ 33.9 │
├────────┼─────┼────────────┼─────────────────┼──────────────┤
But, logic for footer estimation can be done better, i think.
We should have list of columns (with types?), file size, number of rows from iceberg metadata.
Spark for example split by default using 128MB of uncompressed data, we can probably estimate that from types x col_num from one side and using compressed parquet size X ("average/good" parquet compression) from another.
There was a problem hiding this comment.
So may be just increase default const footer size to 2Mb instead of complex logic?
Collaborator
Author
There was a problem hiding this comment.
Yeah, it's one option i keep in mind.
But, for very small files, it probably nice to somewhat scale it down.
Or, for big files, for >2MB estimated footer, we can (would like to) fire 2 requests in parallel, which allow to compress latency as well.
So, i think in the end we can decide on:
- complex estimator, which overestimate with good enough margin footer size.
- very simple estimator based on file size (just to reduce size of estimation for small one)
- just plain request 1-2 MB last bytes
|
AI found a lot of issues for most features, but I think make sense split PR on pieces, one feature in one PR, and recheck these PRs. |
Collaborator
Author
Yes, and it was intendent as such.
I think, it bit early to do that. First we need to find (from bench on real data) which one we really (if any) need. |
`insertRowToLogTable` took the row content as an eagerly-built `String`, so the per-manifest-entry call sites evaluated `getContent(row_index)` (serializing the whole manifest entry) for every data-file entry on every query — even with `iceberg_metadata_log_level = None` (the default), where the result is immediately discarded by the level check inside the function. On wide Iceberg tables with many manifest entries this dominates query planning: on a 17,492-file table the single-threaded manifest pruning spent ~25s (summed across reader threads) building strings that were thrown away. The eager evaluation was the regression that made `iceberg_metadata_processing_threads` look load-bearing — it was mostly parallelizing wasted work. Make the row lazy: `insertRowToLogTable` now takes `std::function<String()>` and invokes it only after the log-level check passes. All six call sites pass a closure instead of a pre-built string. Measured on a synthetic wide table (62x1 GiB files, ~17.5k manifest entries, q reads 8 of 437 columns), warm, single metadata thread: IcebergMetadataReadWaitTimeMicroseconds (summed): 24.8s -> 1.6s (~15x) query wall: 4.09s -> 1.54s (~2.7x) with identical pruning (17,492 files) and identical bytes read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
`estimateParquetFooterSize` used `per_column_chunk = 112` B, which overshot the real thrift-compact ColumnMetaData size. For wide / few-stats-column Iceberg tables the footer is dominated by the `num_columns * per_column_chunk * num_row_groups` term, so the whole prediction ran ~2.2x large: on a 437-column, 8-row-group, ~1 GiB file the predictor asked for 519 KiB when the footer is 240 KiB. That is safe (an overshoot only reads a slightly larger tail) but wasteful. Measured the actual per-chunk cost at ~51 B/chunk on that file (large offsets serialize to ~5 B varints); set `per_column_chunk = 64` (headroom for longer column names) and reduce the safety margin 1.33x -> 1.25x. Validated the predicted-vs-actual footer size across all 62 files of a synthetic wide table (437 cols, 8 row groups): prediction drops from 519 KiB (2.16x) to ~288 KiB (1.17x) with zero underestimates, so the footer is still captured in a single speculative tail read with much less wasted read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The existing `input_format_parquet_hedged_read_threshold_ms` hedges a remote Parquet read once its *total* time exceeds a budget. On object storage that conflates two very different situations: a connection that has stalled (no bytes yet) and a large transfer that is streaming normally but simply takes a while. Budgeting on total time either hedges healthy large reads (wasteful duplicate GETs) or sets the budget so high it no longer cuts the stall tail. Add `input_format_parquet_hedged_read_ttfb_threshold_ms`: fire the hedge when the primary read has not received its *first byte* within the threshold. Time-to-first-byte is largely independent of read size, so this isolates a stalled/slow frontend from a slow-but-progressing transfer. When set (> 0) it takes precedence over the total-time threshold; both remain bounded by `input_format_parquet_hedged_read_max_bytes` and `input_format_parquet_hedged_read_max_inflight`. Mechanism: the primary read now passes a progress callback into `readBigAt` that notifies a new per-task `first_byte` latch on the first reported bytes; `hedgeReadSync` waits on `first_byte` (TTFB mode) instead of `completion`. `first_byte` is also notified unconditionally when the primary finishes, so a read that produced no progress callback (cached region, split path) never leaves a hedge waiting. Measured on a synthetic wide-Iceberg table (62x1 GiB files, q reads ~690 MiB across part-sized S3 GETs, v3 + parallel Iceberg metadata): ttfb_ms p50ms maxms readMiB hedges wins 0(off) 6558 7499 690 0 0 5 6149 6490 1075 158 158 20 5987 6179 1043 133 133 40 5341 5942 749 21 21 At 40 ms: p50 -19%, tail max -21%, +9% bytes (21 hedges, all winning) - the trigger fires only on genuinely stalled frontends. Result is unchanged with the hedge on (identical output hash across thresholds); the loser read is discarded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
This experiment branch (Parquet v3 constant column skip) is not for merge; it exists to get a green build for testing the parquet-v3 read path. The `04401_system_reset_ddl_worker_access` stateless test comes from the antalya-26.6 base backport of ClickHouse#108460 and fails there (the SYSTEM RESET DDL WORKER privilege isn't enforced - unprivileged access is not denied), unrelated to the parquet changes on this branch. Dropping it here unblocks the build. This is NOT a fix for the underlying access-control regression, which lives in the antalya-26.6 base (commit 03e147a) and should be fixed or reverted there; a future merge of that base will reintroduce this test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
…Attributes behind a read setting Reading object metadata on a lake scan (`StorageObjectStorageSource`) needs the object size and etag (the etag keys the page / filesystem / Parquet-metadata caches). The identity fast path fetched these via `GetObjectAttributes`, which also returns the multipart part layout used for part-aligned reads - but that request is heavier than a plain `HEAD` and is issued per file, so a cold, cache-empty scan paid one `GetObjectAttributes` per object (measured ~2x slower cold on a wide-Iceberg benchmark) for part offsets it usually does not use. Default `S3ObjectStorage::getObjectMetadata` to a plain `HEAD` (size + etag, no part offsets), and add a read setting `object_storage_identity_cache_fetch_part_offsets` (default off) that opts back into `GetObjectAttributes` when part-aligned reads want the layout. A new 3-argument `IObjectStorage::getObjectMetadata` overload carries the flag; its default implementation ignores it, so non-S3 storages and the many callers that do not need part offsets are unchanged. The etag is still returned on both paths, so the object-storage caches keep working. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
Resolve conflict in RestCatalog::dropTable: keep the branch's namespace-allowed guard and adopt antalya-26.6's endpoint construction (config.prefix + encodeNamespaceForURI + NAMESPACES_ENDPOINT). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
The parallel manifest-entry pruning hands entries from the producer threads to the consumer through a `ConcurrentBoundedQueue`, one entry at a time. The per-entry work is a tiny min/max prune, so on a wide, many-file table (tens of thousands of manifest entries) the queue's mutex/condvar became the dominant cost: a cold scan spent ~850 ms with the metadata threads ~99% blocked in `__lll_lock_wait` / `__futex` under `IcebergIterator::next` -> `blocking_queue`, doing almost no actual work (the CPU profiler saw ~0 samples there). More threads only added contention, which is why cold was flat across thread counts and no faster than the single-threaded 26.3 path. Hand off entries in batches of `producer_batch_size` (256) instead of one at a time: each producer accumulates into a local vector and pushes a full batch (flushing the partial batch when its work ends), and the consumer serves entries from a locally-held batch under a dedicated `consumer_mutex`, touching the shared queue only once per batch to refill. This amortises the shared-queue lock ~256x, moving the per-entry path onto cheap local buffers. Queue bound is now counted in batches. No change to ordering guarantees (there were none) or results. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: UnamedRus <dtitmoav@gmail.com>
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
3 participants
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...
Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: