Skip to content

Fix racecheck in parquet reader delta byte array kernels - #23898

Open
davidwendt wants to merge 1 commit into
NVIDIA:mainfrom
davidwendt:parquet-racecheck
Open

Fix racecheck in parquet reader delta byte array kernels#23898
davidwendt wants to merge 1 commit into
NVIDIA:mainfrom
davidwendt:parquet-racecheck

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

Description

Fixes some racecheck warnings reported by compute-sanitizer in the weekly CI runs: https://github.com/NVIDIA/cudf/actions/runs/33247027974/job/99086397880#step:5:3314

[ RUN      ] ParquetReaderTest.DeltaByteArrayLargeMiniBlockSkipRows
========= Warning: Race reported between Read access at cudf::io::parquet::detail::<unnamed>::delta_byte_array_decoder::calculate_string_values(unsigned char *, unsigned int, unsigned int, unsigned int)+0x19710 in page_delta_decode.cu:222
=========     and Write access at cudf::io::parquet::detail::<unnamed>::delta_byte_array_decoder::calculate_string_values(unsigned char *, unsigned int, unsigned int, unsigned int)+0x198b0 in page_delta_decode.cu:225 [8 hazards]
=========
========= Warning: Race reported between Read access at cudf::io::parquet::detail::<unnamed>::delta_byte_array_decoder::calculate_string_values(unsigned char *, unsigned int, unsigned int, unsigned int)+0x19710 in page_delta_decode.cu:222
=========     and Write access at cudf::io::parquet::detail::<unnamed>::delta_byte_array_decoder::calculate_string_values(unsigned char *, unsigned int, unsigned int, unsigned int)+0x198b0 in page_delta_decode.cu:225 [16 hazards]
=========
[       OK ] ParquetReaderTest.DeltaByteArrayLargeMiniBlockSkipRows (2385 ms)

Adds appropriate sync-warp to prevent the race condition.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt self-assigned this Aug 31, 2026
@davidwendt
davidwendt requested a review from a team as a code owner August 31, 2026 19:20
@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team labels Aug 31, 2026
@davidwendt
davidwendt requested review from shrshi and ttnghia August 31, 2026 19:20
@davidwendt davidwendt added libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Aug 31, 2026
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fbfbf4f4-7fce-48f7-8643-e0c7b5fbce88

📥 Commits

Reviewing files that changed from the base of the PR and between 6e2d543 and 527ae49.

📒 Files selected for processing (2)
  • cpp/src/io/parquet/delta_binary.cuh
  • cpp/src/io/parquet/page_delta_decode.cu

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when decoding Parquet data with delta-encoded numeric values.
    • Fixed potential race conditions during DELTA_BYTE_ARRAY string reconstruction.
    • Preserved string prefixes correctly across decoded blocks, preventing corrupted or inconsistent results.

Walkthrough

Changes

The Parquet CUDA delta decoders now add warp synchronization around shared state updates. String reconstruction paths snapshot prior string state before updating prefix_seed.

Parquet delta decoder synchronization

Layer / File(s) Summary
Numeric delta state synchronization
cpp/src/io/parquet/delta_binary.cuh
calc_mini_block_pass synchronizes decoded value writes before lane 31 updates last_value.
String reconstruction state synchronization
cpp/src/io/parquet/page_delta_decode.cu
string_scan synchronizes blocker and prefix writes. The mini-batch and character-parallel paths snapshot last_string and its length before copying them into prefix_seed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 527ae

This localized change adjusts synchronization in Parquet byte-array decoding to address reported race warnings; no actionable merge-blocking risk remains.

Suggested reviewers: shrshi, ttnghia, mhaseeb123

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: fixing race-check issues in Parquet reader DELTA_BYTE_ARRAY kernels.
Description check ✅ Passed The description directly explains the race-check warnings, affected kernels, synchronization fix, CI evidence, and test coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

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

Labels

3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants