Skip to content

Parquet v3 read concurrency - #2235

Open
UnamedRus wants to merge 4 commits into
antalya-26.6from
parquet-v3-read-concurrency
Open

Parquet v3 read concurrency#2235
UnamedRus wants to merge 4 commits into
antalya-26.6from
parquet-v3-read-concurrency

Conversation

@UnamedRus

Copy link
Copy Markdown
Collaborator

Number of changes to bring parquet v3 reader perf closer to arrow based

Changelog category (leave one):

  • Performance Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Workflow [PR], commit [ca7a4f6]

Budget memory and threads separately per read stage instead of one shared
fraction, and add a ColumnDataPrefetch stage that issues the compressed
data-page reads (charged to its own memory budget) while ColumnData only
decodes. The old single 0.2 fraction capped the data stage at 0.2 of both
memory and threads, so only ~2 row groups were read/decoded ahead and the S3
link sat idle on latency-bound, high-RTT reads. Now compressed reads run deep
(cheap per row group) while decoded row groups stay bounded, hiding per-GET
latency. Also reconcile the decoded-memory charge up to the actual footprint
inside decodePrimitiveColumn, before formOutputColumn moves the column, so the
honest cap actually bounds decode-ahead.

Squashed extraction of 34816a3 + 114640e + f260506 from the
parquet-v3 feature branch onto antalya-26.6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: UnamedRus <dtitmoav@gmail.com>
@UnamedRus
UnamedRus force-pushed the parquet-v3-read-concurrency branch from ad5f3aa to 79d3658 Compare August 18, 2026 13:46
…2 MiB)

The reader always read a fixed 64 KiB tail to get FileMetaData; files whose
metadata (or metadata + Column/Offset index) exceed that pay a second read.
Size the initial tail to 10% of the file, clamped to [64 KiB, 2 MiB], so the
first read usually covers the whole footer - including the Column/Offset index
just before FileMetaData - without over-reading a large tail on big files.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: UnamedRus <dtitmoav@gmail.com>
@mkmkme

mkmkme commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Could you add Changelog entry please?

mkmkme
mkmkme previously approved these changes Aug 19, 2026

@mkmkme mkmkme left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from the brief look

@UnamedRus

Copy link
Copy Markdown
Collaborator Author

Will do (and benchmark justification), but i would ask to start reviewing of it tomorrow, i want to do some small changes and remove some slop. :)

/// Budget memory and threads separately: a single 0.2 fraction capped ColumnData at 0.2 of both,
/// so only ~2 row groups were read/decoded ahead. Give ColumnData most of the memory and threads
/// (deep, decode-bound); give the small latency-bound index/bloom reads threads for parallelism.
using S = ReadStage;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useless alias. Keeps several characters in each line, but adds cognitive complexity.

set_fractions(S::ColumnDataPrefetch, 0.45, 1);
/// Decode: bounded memory (decoded row groups are large) but most threads. Caps resident decoded
/// row groups independently of prefetch depth.
set_fractions(S::ColumnData, 0.30, 3);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each stage get part of memory/thread budget based on some magic numbers.
I trust that numbers has a ground, but good to add it somewhere.

Under `object_storage_cluster` (distributed object-storage reads), the WHERE
predicate did not reach `ReadFromCluster`, so Iceberg min/max file pruning was
silently skipped (`IcebergMinMaxIndexPrunedFiles=0`) and selective queries
scanned the whole table. Measured on IcebergBench q16: 3.16B vs 78.7M rows,
~7.0s vs ~0.8s (~60x read amplification).

The prune-only `ObjectFilterStep` that carries the predicate to the cluster
task iterator (`getTaskIteratorExtension`) was gated on `use_hive_partitioning`,
so a non-hive Iceberg cluster read got a null filter. Add `ObjectFilterStep`
for any `ReadFromCluster` with a WHERE, not just hive-partitioned tables.
`ObjectFilterStep::updatePipeline` is a no-op, so it never filters rows on the
initiator -- required at `WithMergeableState`, where the filter columns may be
absent from the blocks returned by cluster replicas.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ter to physical column names

The prior fix delivered the WHERE predicate to ReadFromCluster's ObjectFilterStep,
but its DAG inputs are analyzer column identifiers (e.g. `__table1.date`) while the
Iceberg pruner resolves against physical schema names (`date`). So
`tryGetColumnIDByName` returned nullopt for every input, no min/max key conditions
were built, and `IcebergMinMaxIndexPrunedFiles` stayed 0 (full-table over-read on
selective cluster queries).

On the single-node (FetchColumns) path this works only because a
"change names to identifiers" ExpressionStep sits below the WHERE FilterStep and
optimizePrimaryKeyConditionAndLimit merges the filter through it (identifiers ->
names). At WithMergeableState (the cluster path) that step is absent.

Rewrite the ObjectFilterStep's DAG in addObjectFilterStep: build a rename DAG whose
inputs are physical names aliased to the identifiers, then ActionsDAG::merge the
filter through it, so the DAG that reaches getTaskIteratorExtension ->
IcebergMetadata::iterate -> ManifestFileIterator carries physical column names and
the min/max pruner resolves them. Mirrors the single-node rename mechanism.

Analyzer path only (the non-analyzer InterpreterSelectQuery path is not supported).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants