[GLUTEN-8455][VL] Fallback Scan for Encrypted Parquet Files #8456

ArnavBalyan · 2025-01-07T19:55:37Z

Currently Scan is offloaded to Velox, however Velox does not support encrypted parquet files.
This leads to a Velox side error and runtime failure. Added support to fallback in such cases.
We attempt to read parquet footer for the root paths in scan operator, if decryption is detected, the scan fallsback
This is behind a config and will not be enabled by default, users can enable scan fallback by passing spark.gluten.sql.fallbackEncryptedParquet true.

Fixes: #8455

github-actions · 2025-01-07T19:56:01Z

#8455

github-actions · 2025-01-07T19:56:15Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-01-07T20:17:38Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-01-07T20:18:18Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-01-07T20:39:00Z

Run Gluten Clickhouse CI on x86

Yohahaha

LGTM! just one comments

Yohahaha · 2025-01-08T11:11:30Z

shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala

@@ -2278,4 +2281,11 @@ object GlutenConfig {
        "Otherwise, broadcast build relation will use onheap memory.")
      .booleanConf
      .createWithDefault(false)
+
+  val ENCRYPTED_PARQUET_FALLBACK_ENABLED =
+    buildStaticConf("spark.gluten.sql.fallbackEncryptedParquet")


why use static conf?

updated, this config changing at runtime is an unlikely event, kept removed static to allow any such usecases

jackylee-ch · 2025-01-08T12:05:01Z

backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala

-      case _ => ValidationResult.succeeded
+    def validateEncryption(): Option[String] = {
+
+      val encryptionValidationEnabled = GlutenConfig.getConf.enableEncryptedParquetFallback


The config name is a little wird to me. maybe parquetEncryptionValidationEnabled?

jackylee-ch · 2025-01-08T12:10:35Z

backends-velox/src/main/scala/org/apache/gluten/utils/ParquetMetadataUtils.scala

+      val fileStatus = filesIterator.next()
+      checkedFileCount += 1
+      try {
+        ParquetFileReader.readFooter(conf, fileStatus.getPath).toString


Is it a better way to check encrypted metadata than use the Exception?

This is done to keep it backward compatible, spark 33 uses parquet 1.12, support is not there yet

Spark 3.5 uses parquet 1.13, and I found it defined in thrift. This could work for Spark 3.5?

github-actions · 2025-01-08T19:04:28Z

Run Gluten Clickhouse CI on x86

ArnavBalyan · 2025-01-08T19:06:32Z

Thanks for the review @Yohahaha addressed comments, could you please take a look

FelixYBW · 2025-01-08T21:24:08Z

CI fails. can you fix format?

github-actions · 2025-01-08T21:38:30Z

Run Gluten Clickhouse CI on x86

Yohahaha · 2025-01-09T01:27:48Z

[ERROR] /__w/incubator-gluten/incubator-gluten/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala:196: error: value getConf is not a member of object org.apache.gluten.config.GlutenConfig
[ERROR]       val encryptionValidationEnabled = GlutenConfig.getConf.parquetEncryptionValidationEnabled
[ERROR]                                                      ^
[ERROR] one error found
[ERROR] exception compilation error occurred!!!

please rebase and fix conflict.

github-actions · 2025-01-09T04:46:15Z

Run Gluten Clickhouse CI on x86

jackylee-ch · 2025-01-09T07:49:04Z

@Yohahaha The way to check encrypted file haven't been confirmed, I'm afraid this is not work on Spark 3.5

ArnavBalyan · 2025-01-09T08:07:57Z

@Yohahaha The way to check encrypted file haven't been confirmed, I'm afraid this is not work on Spark 3.5

Hi @jackylee-ch, spark 3.5 uses parquet 1.13. After parquet 1.13, there is a new field added to check for encryption, which can provide if the file is encrypted. However if we try to read the encrypted file footer, it throws ParquetCryptoRuntimeException. Could you please elaborate on why it may not work on 3.5? Thanks

Alternatively, I was planning to shade parquet 1.14 and bring the shaded parquet as a packaged dependency in Gluten (to check the encryption), which will be a more elegant solution and work with multiple spark version. If there are no concerns with bringing in shaded parquet inside gluten, I'm happy to work on that implementation as well. I could not see such implementations inside Gluten so was a bit hesitant, althought parquet encryption seems to be a special case and could benefit from such a solution. Let me know what you think thanks!

Yohahaha · 2025-01-09T08:12:31Z

@Yohahaha The way to check encrypted file haven't been confirmed, I'm afraid this is not work on Spark 3.5

@jackylee-ch sorry, I missed your comments.

the check is disabled by default, it's safe to merge to main branch. if we verified it's not work in Spark 3.5, we can fix with a followup PR.

Yohahaha · 2025-01-09T08:16:03Z

@ArnavBalyan thanks for the update!

would you add UT in different spark shim module with a minimal encrypted parquet file?

we do not need package parquet deps in Gluten I think, just do verification in shim module.

Yohahaha · 2025-01-09T08:18:15Z

@jackylee-ch I'm ok if you want to revert this PR with Spark 3.5 compatibility concern.

jackylee-ch · 2025-01-09T08:22:28Z

Hi @jackylee-ch, spark 3.5 uses parquet 1.13. After parquet 1.13, there is a new field added to check for encryption, which can provide if the file is encrypted. However if we try to read the encrypted file footer, it throws ParquetCryptoRuntimeException. Could you please elaborate on why it may not work on 3.5? Thanks

@ArnavBalyan You mean that no matter which Spark version we use, we can get ParquetCryptoRuntimeException if I try to read the encrypted footer?

BTW what would happen if the footer is not encrypted but the column is encrypted?

jackylee-ch · 2025-01-09T08:30:54Z

@jackylee-ch I'm ok if you want to revert this PR with Spark 3.5 compatibility concern.

Current PR should work fine for Spark 3.2 and 3.3 for encrypted footer, not sure for other cases. If it doesn't support those cases, we need move ParquetMetadataUtils to shim layer and support other cases.

Since it is merged and not work by default, a followed PR is good for me.

ArnavBalyan · 2025-01-09T08:38:33Z

Hi @jackylee-ch, spark 3.5 uses parquet 1.13. After parquet 1.13, there is a new field added to check for encryption, which can provide if the file is encrypted. However if we try to read the encrypted file footer, it throws ParquetCryptoRuntimeException. Could you please elaborate on why it may not work on 3.5? Thanks

@ArnavBalyan You mean that no matter which Spark version we use, we can get ParquetCryptoRuntimeException if I try to read the encrypted footer?

BTW what would happen if the footer is not encrypted but the column is encrypted?

Sure let me add a follow up UT for 3.5, this feature is behind feature flag and verified for parquet 1.13. @jackylee-ch, it would depend how you are doing encryption in your setup. Typically the footer metadata will indicate encryption for newer versions of parquet.

we do not need package parquet deps in Gluten I think, just do verification in shim module.

@Yohahaha, if we want to keep it backward compatible for parquet 1.12, and not use exception, then we will need a newer parquet inside Gluten regardless of the spark version, in that case, the check can hold true if there are future parquet upgrades inside spark. Using the shim layer we can probably do validation but still will use exception checking for 1.12, I was wondering how feasible it would be to bring in shaded parquet to do this? Thanks

jackylee-ch · 2025-01-09T09:03:54Z

We can use different checker for different Spark version in shims, and for parquet 1.12, the exception is good to me if there is no other better idea.

github-actions bot added CORE works for Gluten Core VELOX labels Jan 7, 2025

ArnavBalyan added 2 commits January 7, 2025 20:16

update

497d52b

update

e375c06

ArnavBalyan force-pushed the arnavb/encrypted-parquet-fallback branch from dab4674 to e375c06 Compare January 7, 2025 20:17

update

7aa3331

rebase

a152ba8

Yohahaha approved these changes Jan 8, 2025

View reviewed changes

jackylee-ch reviewed Jan 8, 2025

View reviewed changes

update

b666e05

update

5173dce

rebase

e8e66c4

Yohahaha merged commit de725bd into apache:main Jan 9, 2025
48 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GLUTEN-8455][VL] Fallback Scan for Encrypted Parquet Files #8456

[GLUTEN-8455][VL] Fallback Scan for Encrypted Parquet Files #8456

ArnavBalyan commented Jan 7, 2025 •

edited by Yohahaha

Loading

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

Yohahaha left a comment

Yohahaha Jan 8, 2025

ArnavBalyan Jan 8, 2025

jackylee-ch Jan 8, 2025

ArnavBalyan Jan 8, 2025

jackylee-ch Jan 8, 2025

ArnavBalyan Jan 8, 2025

jackylee-ch Jan 9, 2025

github-actions bot commented Jan 8, 2025

ArnavBalyan commented Jan 8, 2025

FelixYBW commented Jan 8, 2025

github-actions bot commented Jan 8, 2025

Yohahaha commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

jackylee-ch commented Jan 9, 2025

ArnavBalyan commented Jan 9, 2025 •

edited

Loading

Yohahaha commented Jan 9, 2025

Yohahaha commented Jan 9, 2025

Yohahaha commented Jan 9, 2025

jackylee-ch commented Jan 9, 2025 •

edited

Loading

jackylee-ch commented Jan 9, 2025 •

edited

Loading

ArnavBalyan commented Jan 9, 2025 •

edited

Loading

jackylee-ch commented Jan 9, 2025

[GLUTEN-8455][VL] Fallback Scan for Encrypted Parquet Files #8456

[GLUTEN-8455][VL] Fallback Scan for Encrypted Parquet Files #8456

Conversation

ArnavBalyan commented Jan 7, 2025 • edited by Yohahaha Loading

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

Yohahaha left a comment

Choose a reason for hiding this comment

Yohahaha Jan 8, 2025

Choose a reason for hiding this comment

ArnavBalyan Jan 8, 2025

Choose a reason for hiding this comment

jackylee-ch Jan 8, 2025

Choose a reason for hiding this comment

ArnavBalyan Jan 8, 2025

Choose a reason for hiding this comment

jackylee-ch Jan 8, 2025

Choose a reason for hiding this comment

ArnavBalyan Jan 8, 2025

Choose a reason for hiding this comment

jackylee-ch Jan 9, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 8, 2025

ArnavBalyan commented Jan 8, 2025

FelixYBW commented Jan 8, 2025

github-actions bot commented Jan 8, 2025

Yohahaha commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

jackylee-ch commented Jan 9, 2025

ArnavBalyan commented Jan 9, 2025 • edited Loading

Yohahaha commented Jan 9, 2025

Yohahaha commented Jan 9, 2025

Yohahaha commented Jan 9, 2025

jackylee-ch commented Jan 9, 2025 • edited Loading

jackylee-ch commented Jan 9, 2025 • edited Loading

ArnavBalyan commented Jan 9, 2025 • edited Loading

jackylee-ch commented Jan 9, 2025

ArnavBalyan commented Jan 7, 2025 •

edited by Yohahaha

Loading

ArnavBalyan commented Jan 9, 2025 •

edited

Loading

jackylee-ch commented Jan 9, 2025 •

edited

Loading

jackylee-ch commented Jan 9, 2025 •

edited

Loading

ArnavBalyan commented Jan 9, 2025 •

edited

Loading