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.

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

[FEA] Improve exception message when unknown Parquet page encoding detected #14209

Closed
jlowe opened this issue Sep 27, 2023 · 16 comments
Closed
Assignees
Labels
0 - Backlog In queue waiting for assignment cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Spark Functionality that helps Spark RAPIDS

Comments

@jlowe
Copy link
Member

jlowe commented Sep 27, 2023

Is your feature request related to a problem? Please describe.
A user of the RAPIDS Accelerator for Apache Spark reported the following exception:

ai.rapids.cudf.CudfException: CUDF failure at: /home/jenkins/agent/workspace/jenkins-spark-rapids-jni_nightly-dev-424-cuda11/thirdparty/cudf/cpp/src/io/parquet/reader_impl_preprocess.cu:346: Unsupported page encoding detected
	at ai.rapids.cudf.ParquetChunkedReader.readChunk(Native Method)
	at ai.rapids.cudf.ParquetChunkedReader.readChunk(ParquetChunkedReader.java:111)
	at com.nvidia.spark.rapids.ParquetTableReader.$anonfun$next$1(GpuParquetScan.scala:2639)
	at com.nvidia.spark.rapids.Arm$.withResource(Arm.scala:29)
	at com.nvidia.spark.rapids.ParquetTableReader.next(GpuParquetScan.scala:2638)
	at com.nvidia.spark.rapids.ParquetTableReader.next(GpuParquetScan.scala:2615)
	at com.nvidia.spark.rapids.CachedGpuBatchIterator$.$anonfun$apply$1(GpuDataProducer.scala:161)
	at com.nvidia.spark.rapids.Arm$.withResource(Arm.scala:29)
	at com.nvidia.spark.rapids.CachedGpuBatchIterator$.apply(GpuDataProducer.scala:158

The exception message from libcudf is not very helpful in that it says an unsupported page encoding was detected but not what that unexpected page encoding was (i.e.: the enum value). Without this information, we're left guessing what encoding was found in the file and usually have to request users to share a sample file to find out. Not all users are willing to share sample files.

Describe the solution you'd like
Exception messages for an unexpected/unsupported value should show the value as part of the exception message.

@jlowe jlowe added feature request New feature or request Needs Triage Need team to review and classify libcudf Affects libcudf (C++/CUDA) code. cuIO cuIO issue Spark Functionality that helps Spark RAPIDS labels Sep 27, 2023
@GregoryKimball GregoryKimball moved this to Needs owner in libcudf Oct 12, 2023
@GregoryKimball GregoryKimball added 0 - Backlog In queue waiting for assignment and removed Needs Triage Need team to review and classify labels Oct 12, 2023
@ZelboK
Copy link
Contributor

ZelboK commented Nov 15, 2023

Hi, I'd like to try doing this task.

@GregoryKimball
Copy link
Contributor

Thank you @ZelboK. It's possible that #14237 partially or fully addressed this. @etseidl would you please share your thoughts?

@etseidl
Copy link
Contributor

etseidl commented Nov 15, 2023

This has not yet been fully addressed. Right now the error code for unsupported option is set during header parsing, and available on the host side in decode_page_headers. All that needs to be done is to test the returned error code for the correct bit, then do a transform_reduce on the page encoding field (after turning the encoding into a mask), and then check the set bits to find the offending encoding(s). Soon the only unsupported encoding will be byte_stream_split, so this hasn't been a high priority for me. Good first issue.

@vuule
Copy link
Contributor

vuule commented Nov 15, 2023

I think @nvdbaranec 's #14360 makes this easy to address; see

// validate page encodings
CUDF_EXPECTS(std::all_of(pass.pages.begin(),
pass.pages.end(),
[](auto const& page) { return is_supported_encoding(page.encoding); }),
"Unsupported page encoding detected");
}

@etseidl
Copy link
Contributor

etseidl commented Nov 15, 2023

That's zombie code that was removed in #14237 😅 (https://github.com/rapidsai/cudf/pull/14237/files#diff-ebcd42136ddf31f8097631e00ca84c03abf1fa38f1eb1afbc50d3d207c7a7cac). Looks like it was missed in a merge somwhere along the line.

@vuule
Copy link
Contributor

vuule commented Nov 15, 2023

??
You just moved it to https://github.com/rapidsai/cudf/blob/branch-23.12/cpp/src/io/parquet/parquet_gpu.hpp#L60
Please stop spreading panic :D

@etseidl
Copy link
Contributor

etseidl commented Nov 15, 2023

I was talking about the std:all_of call 😉 That's my old detection code from #12754.

@ZelboK
Copy link
Contributor

ZelboK commented Nov 17, 2023

Hi, from what I can see 24.02 and 23.12 have both resolved this issue. Could I get some clarification on what needs to be done for this issue?

@etseidl
Copy link
Contributor

etseidl commented Nov 17, 2023

@ZelboK the current state of affairs is that when an unsupported encoding is detected, an error message is printed with an error code, which is a mask comprising the union of all errors detected. The current list of errors is here. What is desired is to print a list of which encodings triggered the message.

@ZelboK
Copy link
Contributor

ZelboK commented Nov 17, 2023

Ah I see. I was fixated on the old CUDF_EXPECTS and std::all code (which is no longer there) so I skimmed through the new code. But now i see that there is still work to be done. I apologize, I end up having to do a lot of context switching so I missed this.

Thanks

@ZelboK
Copy link
Contributor

ZelboK commented Dec 20, 2023

@vuule @etseidl Hey folks, just wanted to know if there were more tasks I could do in cuIO, now that this one is done? I imagine most of the team will be on vacation if not already so. Just looking to learn more on my holidays, I don't mind more challenging tasks. Thanks!

Also my apologies, I am assuming you two are on the cuIO side of things. Forgive me if I am wrong 😅

rapids-bot bot pushed a commit that referenced this issue Dec 20, 2023
…oding is detected (#14453)

Per #14209 this will list out unsupported encodings that were found.

Authors:
  - Danial Javady (https://github.com/ZelboK)
  - Vukasin Milovanovic (https://github.com/vuule)
  - Nghia Truong (https://github.com/ttnghia)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Vukasin Milovanovic (https://github.com/vuule)
  - Ed Seidl (https://github.com/etseidl)

URL: #14453
@vuule
Copy link
Contributor

vuule commented Dec 20, 2023

Thank you for offering further help!
If you're interesting you can work on #14661.
Otherwise feel free to pick any unassigned issue in https://github.com/rapidsai/cudf/issues?q=is%3Aissue+is%3Aopen++label%3Acuio+label%3A%22good+first+issue%22
Just let me know so I can assign :)

@ZelboK
Copy link
Contributor

ZelboK commented Dec 20, 2023

Hm,

Thank you for offering further help! If you're interesting you can work on #14661. Otherwise feel free to pick any unassigned issue in https://github.com/rapidsai/cudf/issues?q=is%3Aissue+is%3Aopen++label%3Acuio+label%3A%22good+first+issue%22 Just let me know so I can assign :)

Hm, would #14661 be a more challenging one? From what I see in that list, they are all labeled good first issues, I'd actually like to take something more challenging and avoid first issues. Honestly the harder the better. I'm hoping to learn as much as I can in the next few weeks so I aiming to actually do a few more

@vuule
Copy link
Contributor

vuule commented Dec 20, 2023

None of the above are trivial, and many open issues are still open for a reason 😅
If you want to dig deeper into the code (but maybe not too deep), I can suggest #13837
You can also explore https://github.com/rapidsai/cudf/issues?page=1&q=is%3Aissue+is%3Aopen++label%3Acuio

@DanialJavady96
Copy link
Contributor

None of the above are trivial, and many open issues are still open for a reason 😅 If you want to dig deeper into the code (but maybe not too deep), I can suggest #13837 You can also explore https://github.com/rapidsai/cudf/issues?page=1&q=is%3Aissue+is%3Aopen++label%3Acuio

Thank you so much! I assumed that the good first issue label was for easier work. In that case, I'll do #14661 and depending on how that goes, hopefully I can pick a few more from the backlog. Thanks again :)

@vuule
Copy link
Contributor

vuule commented Dec 20, 2023

Should be addressed by #14453. @jlowe please close if the new error message is works well for you.

@jlowe jlowe closed this as completed Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Backlog In queue waiting for assignment cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Spark Functionality that helps Spark RAPIDS
Projects
Archived in project
Development

No branches or pull requests

6 participants