change gharchive query to reduce memory blowup, disable Delta #5026
Conversation
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
Contributor
|
Does it seem like datafusion loads all columns in this case? I wonder if it wants the null counts for all arrays. |
Contributor
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb S3Summary
Detailed Results Table
|
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Benchmarks: TPC-H SF=1 on S3Summary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-DS SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on S3Summary
Detailed Results Table
|
Contributor
Benchmarks: Clickbench on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: Statistical and Population GeneticsSummary
Detailed Results Table
|
Contributor
Author
|
Oh huh, we don't push the filter down at all apparently 
I have been under the mistaken impression that we are able to. I see also #5024 |
Contributor
Author
|
Merging @asubiotto's change helps a lot here. I'm going to review and try and merge that and then revisit this |
a10y
force-pushed
the
aduffy/fix-gharchive-crash
branch
2 times, most recently
from
09ed4ff to
b426be7
Compare
Contributor
Benchmarks: GitHub Archive (NVMe)Summary
Detailed Results Table
|
Contributor
Benchmarks: GitHub Archive (S3)Summary
Detailed Results Table
|
Contributor
|
Why are we disabling RLE? |
Contributor
I'm pretty sure at this point the culprit is delta. Applying this diff running in debug, did not trigger the issue for GHArchive. The difference being that the indices are not delta encoded here yet. If the issue would be within RLE, this should also trigger the out of bounds access in my understanding. I did this in encode to see the uncompressed pritimitive array in case the assertion fires. |
Contributor
Author
|
I can just disable Delta if that is preferable |
0ax1
reviewed
Oct 22, 2025
Contributor
|
The new query succeeds by the way without triggering an out of bounds, even with Delta + RLE enabled. Just ran this locally. |
Contributor
Author
|
That's because the query is no longer decoding the array that needs to be decoded to trigger #4973 , but the data is still being written corrupted |
Contributor
Yeah makes sense |
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
a10y
force-pushed
the
aduffy/fix-gharchive-crash
branch
from
47e3ec3 to
6f491c6
Compare
a10y
changed the title
gatesn
approved these changes
Oct 22, 2025
Contributor
gatesn
left a comment
gatesn
left a comment
There was a problem hiding this comment.
Let's do it as temporary fix, then can bring the tests back
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
robert3005
approved these changes
Oct 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Disable RLE scheme, change
select *toselect count(*). That in combination with #5024 stops us from OOMing every time we run this benchmark.