Skip to content
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

Update buffer store to return compressed batches directly, add compression NVTX ranges #572

Merged
merged 3 commits into from
Aug 19, 2020
Merged

Update buffer store to return compressed batches directly, add compression NVTX ranges #572

merged 3 commits into from
Aug 19, 2020

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Aug 17, 2020

Updates getColumnarBatch for RapidsBuffer so it will not dynamically decompress batches but instead return the batch "as-is", either compressed or uncompressed. This properly connects the shuffle reader to the batch decompressor, as it was decompressing buffers piecemeal before rather than batching.

This also fixes a resource leak after batch decompression where it was forgetting to close the device buffer after building the columnar batch.

@jlowe
Update buffer store to return compressed batches directly, add compre…
fc7fc22 
…ssion NVTX ranges

Signed-off-by: Jason Lowe <jlowe@nvidia.com>
@jlowe jlowe added bug Something isn't working shuffle things that impact the shuffle plugin labels Aug 17, 2020
@jlowe jlowe added this to the Aug 17 - Aug 28 milestone Aug 17, 2020
@jlowe jlowe self-assigned this Aug 17, 2020
@jlowe
Copy link
Member Author

jlowe commented Aug 17, 2020

build

abellina
abellina previously approved these changes Aug 19, 2020
View reviewed changes
Copy link
Collaborator

@abellina abellina left a comment

Choose a reason for hiding this comment

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

Just had a nit, otherwise it looks good.

@jlowe
Copy link
Member Author

jlowe commented Aug 19, 2020

build

@jlowe jlowe merged commit 123a9c9 into NVIDIA:branch-0.2 Aug 19, 2020
pxLi added a commit to pxLi/spark-rapids that referenced this pull request Aug 24, 2020
@pxLi @andygrove
@tgravescs @nartal1 @jlowe @revans2
Local changelog test (#32)
b609d99 
* explicitly disable AQE in one test (NVIDIA#567)

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Enable using spark-submit to convert from csv to parquet (NVIDIA#565)

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* xfail the Spark 3.1.0 integration tests that fail  (NVIDIA#580)

* xfail GpuTimeSub, arithmetic ops, and full outer join failures on 3.1.0

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* xfail the rest of the 3.1.0 tests and enable 3.1.0 unit tests in the
jenkins builds

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Fix unit tests when AQE is enabled (NVIDIA#558)

* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* fix indentation

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* Update buffer store to return compressed batches directly, add compression NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* xfail the tpch spark 3.1.0 tests that fail (NVIDIA#588)

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Move GpuParquetScan/GpuOrcScan into Shim (NVIDIA#590)

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Fixes

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Fix imports

Signed-off-by: Thomas Graves <tgraves@apache.org>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Filter nulls from joins where possible to improve performance. (NVIDIA#594)

* Filter nulls from joins where possible to improve performance.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>

* Addressed review comments

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <pxli@nyu.edu>

* only filter out labels for issue

* add nightly workflow on github actions

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>
Co-authored-by: Thomas Graves <tgraves@apache.org>
Co-authored-by: Thomas Graves <tgraves@nvidia.com>
Co-authored-by: Niranjan Artal <50492963+nartal1@users.noreply.github.com>
Co-authored-by: Jason Lowe <jlowe@nvidia.com>
Co-authored-by: Robert (Bobby) Evans <bobby@apache.org>
pxLi added a commit to pxLi/spark-rapids that referenced this pull request Aug 24, 2020
@pxLi @andygrove
@tgravescs @nartal1 @jlowe @revans2
Local changelog test (#33)
bbbeadb 
* explicitly disable AQE in one test (NVIDIA#567)

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Enable using spark-submit to convert from csv to parquet (NVIDIA#565)

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* xfail the Spark 3.1.0 integration tests that fail  (NVIDIA#580)

* xfail GpuTimeSub, arithmetic ops, and full outer join failures on 3.1.0

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* xfail the rest of the 3.1.0 tests and enable 3.1.0 unit tests in the
jenkins builds

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Fix unit tests when AQE is enabled (NVIDIA#558)

* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* fix indentation

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* Update buffer store to return compressed batches directly, add compression NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* xfail the tpch spark 3.1.0 tests that fail (NVIDIA#588)

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Move GpuParquetScan/GpuOrcScan into Shim (NVIDIA#590)

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Fixes

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Fix imports

Signed-off-by: Thomas Graves <tgraves@apache.org>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* Filter nulls from joins where possible to improve performance. (NVIDIA#594)

* Filter nulls from joins where possible to improve performance.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>

* Addressed review comments

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <pxli@nyu.edu>

* only filter out labels for issue

* add nightly workflow on github actions

* fix format

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>
Co-authored-by: Thomas Graves <tgraves@apache.org>
Co-authored-by: Thomas Graves <tgraves@nvidia.com>
Co-authored-by: Niranjan Artal <50492963+nartal1@users.noreply.github.com>
Co-authored-by: Jason Lowe <jlowe@nvidia.com>
Co-authored-by: Robert (Bobby) Evans <bobby@apache.org>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@jlowe
Update buffer store to return compressed batches directly, add compre…
1c44a16 
…ssion NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@jlowe
Update buffer store to return compressed batches directly, add compre…
2cb1b22 
…ssion NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>
@jlowe jlowe deleted the fix-batch-decompress branch September 10, 2021 15:31
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Update submodule cudf to a270ae6303487397a21ade949786c43636194752 (NV…
78df140 
…IDIA#572)

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abellina abellina abellina approved these changes

Assignees

@jlowe jlowe

Labels
bug Something isn't working shuffle things that impact the shuffle plugin
Projects
None yet
Milestone
Aug 17 - Aug 28
Development

Successfully merging this pull request may close these issues.
2 participants
@jlowe @abellina