Skip to content

Move scan to use handles #4646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
gatesn merged 52 commits into from
Sep 16, 2025
Merged

Move scan to use handles #4646

gatesn merged 52 commits into from
Sep 16, 2025

Conversation

@gatesn
Copy link
Contributor

@gatesn gatesn commented Sep 15, 2025 •
edited
Loading

Note, there are some regressions in DuckDB NVMe queries that use dynamic filtering. I would like to fix these in a FLUP since they may impact all scans (incl DataFusion).

FLUPs:

  • Fix dynamic filtering in DuckDB scans
  • Move each language API over to a CurrentThreadRuntime with a configurable number of background worker threads.

gatesn added 30 commits September 13, 2025 08:59
@gatesn
File Read/Write
0b6f4aa 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
File Read/Write
9cd6681 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
File Read/Write
bdf9ecf 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
File Read/Write
ab37b27 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
File Read/Write
17e504e 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
96c136a 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
8086c04 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
accb278 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
a257aed 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
a2b9fc2 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Merge develop
cc0071d 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Convert handles to hold weak reference to their runtime
fbf3d6a 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Convert handles to hold weak reference to their runtime
7f69653 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Convert handles to hold weak reference to their runtime
286f3d1 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Convert handles to hold weak reference to their runtime
b3782fd 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Convert handles to hold weak reference to their runtime
5533a21 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Rebase
07e96ea
@gatesn
Rebase
ac952ac
@gatesn
Rebase
88f2b5c
@gatesn
Merge branch 'ngates/weak-handle' into ngates/read-write
bf8fd9a
@gatesn
More things
d55ec8e 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
More things
1e6d2ca 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
More things
1fa9672 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
More things
8547bad 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
More things
fea3a62 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Make VortexReadAt return static futures
c667922 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Make VortexReadAt return static futures
19554c7 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Make VortexReadAt return static futures
217e0a8 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Make VortexReadAt return static futures
c922950 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Make VortexReadAt return static futures
2aed700 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Scan with handles
e30f84f 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn
Scan with handles
ba67956 
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn gatesn added the action/benchmark Trigger full benchmarks to run on this PR label Sep 16, 2025
@github-actions github-actions bot removed the action/benchmark Trigger full benchmarks to run on this PR label Sep 16, 2025
@gatesn gatesn requested a review from onursatici September 16, 2025 10:12
@onursatici onursatici added the action/benchmark Trigger full benchmarks to run on this PR label Sep 16, 2025
@github-actions github-actions bot removed the action/benchmark Trigger full benchmarks to run on this PR label Sep 16, 2025
onursatici
onursatici approved these changes Sep 16, 2025
View reviewed changes
vortex-duckdb/src/lib.rs
// **WARNING begin this includes duckdb-rs, which is required to link in the symbol from libduckdb-sys.
use tokio::runtime;
use tokio::runtime::Runtime;
// **WARNING end
Copy link
Contributor

@onursatici onursatici Sep 16, 2025

Choose a reason for hiding this comment

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

Suggested change
// **WARNING end

@onursatici
Copy link
Contributor

onursatici commented Sep 16, 2025

Just so I understand, the slowdown in duckdb comes from missing work stealing that we have to bring back in the different form under new runtime?

Because the same iterator is shared among all worker threads that use the same current thread runtime, there is now one queue for tasks that all threads poll from

@gatesn gatesn merged commit 156a94f into develop Sep 16, 2025
65 checks passed
@gatesn gatesn deleted the ngates/scan-handles branch September 16, 2025 11:44
@robert3005
Copy link
Contributor

robert3005 commented Sep 16, 2025

Looks like recent commits made the slowdown go away and we have some nice improvements as well!

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

Reviewers

@onursatici onursatici onursatici approved these changes

Assignees

No one assigned

Labels

changelog/feature A new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gatesn @robert3005 @onursatici