Skip to content

[VL] Make ValueStreamDataSource::next() iterative instead of recursive - #13346

Open
malinjawi wants to merge 1 commit into
apache:mainfrom
malinjawi:valuestream-next-iterative
Open

malinjawi wants to merge 1 commit into
apache:mainfrom
malinjawi:valuestream-next-iterative

Conversation

@malinjawi

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Revives #12563, which the stale bot closed without review. Rebased onto main.

With spark.gluten.sql.columnar.backend.velox.valueStream.dynamicFilter.enabled=true (#11657, off by default), ValueStreamDataSource::next() calls itself once for every batch the dynamic filter removes entirely, so a long run of such batches grows the native stack and can overflow the executor thread. This makes the retry a loop, with no other behavior change. It's needed before the filter from #11605 can be on by default.

How was this patch tested?

A new manyConsecutiveFullyFilteredBatches case in ValueStreamDynamicFilterTest (velox_operators_test) filters out 4096 consecutive batches in one split and checks that the batch after them still comes through.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: IBM Bob

cc @acvictor @zhztheplayer

ValueStreamDataSource::next() retried by calling itself. Each exhausted
iterator, end-of-stream batch, or batch fully eliminated by a dynamic
filter consumed a stack frame, so with a selective dynamic filter pushed
into a value stream (valueStream.dynamicFilter.enabled), a long run of
eliminated batches grew the native stack in proportion to the number of
skipped batches.

The retry logic is now a loop. Every branch keeps its previous
semantics: an exhausted stream and a null batch both release the current
iterator before retrying, a fully-filtered batch retries on the same
iterator, and an empty pending-iterator queue still returns an engaged
null RowVectorPtr so TableScan reads it as "current split finished"
rather than connector-blocked.

Adds a regression test that pushes a dynamic filter eliminating 4096
consecutive batches through a single split.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant