You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/19 13:35:26 UTC

[GitHub] [spark] hezuojiao opened a new pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

hezuojiao opened a new pull request #31898:
URL: https://github.com/apache/spark/pull/31898


   ### What changes were proposed in this pull request?
   
   This patch proposes to disable fetching shuffle blocks in batch when io encryption is enabled. Adaptive Query Execution fetch contiguous shuffle blocks for the same map task in batch to reduce IO and improve performance. However, we found that batch fetching is incompatible with io encryption. 
   
   ### Why are the changes needed?
   Before this patch, we set `spark.io.encryption.enabled` to true, then run some queries which coalesced partitions by AEQ, may got following error message:
   ```14:05:52.638 WARN org.apache.spark.scheduler.TaskSetManager: Lost task 1.0 in stage 2.0 (TID 3) (11.240.37.88 executor driver): FetchFailed(BlockManagerId(driver, 11.240.37.88, 63574, None), shuffleId=0, mapIndex=0, mapId=0, reduceId=2, message=
   org.apache.spark.shuffle.FetchFailedException: Stream is corrupted
   	at org.apache.spark.storage.ShuffleBlockFetcherIterator.throwFetchFailedException(ShuffleBlockFetcherIterator.scala:772)
   	at org.apache.spark.storage.BufferReleasingInputStream.read(ShuffleBlockFetcherIterator.scala:845)
   	at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
   	at java.io.BufferedInputStream.read(BufferedInputStream.java:265)
   	at java.io.DataInputStream.readInt(DataInputStream.java:387)
   	at org.apache.spark.sql.execution.UnsafeRowSerializerInstance$$anon$2$$anon$3.readSize(UnsafeRowSerializer.scala:113)
   	at org.apache.spark.sql.execution.UnsafeRowSerializerInstance$$anon$2$$anon$3.next(UnsafeRowSerializer.scala:129)
   	at org.apache.spark.sql.execution.UnsafeRowSerializerInstance$$anon$2$$anon$3.next(UnsafeRowSerializer.scala:110)
   	at scala.collection.Iterator$$anon$11.next(Iterator.scala:494)
   	at scala.collection.Iterator$$anon$10.next(Iterator.scala:459)
   	at org.apache.spark.util.CompletionIterator.next(CompletionIterator.scala:29)
   	at org.apache.spark.InterruptibleIterator.next(InterruptibleIterator.scala:40)
   	at scala.collection.Iterator$$anon$10.next(Iterator.scala:459)
   	at org.apache.spark.sql.execution.SparkPlan.$anonfun$getByteArrayRdd$1(SparkPlan.scala:345)
   	at org.apache.spark.rdd.RDD.$anonfun$mapPartitionsInternal$2(RDD.scala:898)
   	at org.apache.spark.rdd.RDD.$anonfun$mapPartitionsInternal$2$adapted(RDD.scala:898)
   	at org.apache.spark.rdd.MapPartitionsRDD.compute(MapPartitionsRDD.scala:52)
   	at org.apache.spark.rdd.RDD.computeOrReadCheckpoint(RDD.scala:373)
   	at org.apache.spark.rdd.RDD.iterator(RDD.scala:337)
   	at org.apache.spark.scheduler.ResultTask.runTask(ResultTask.scala:90)
   	at org.apache.spark.scheduler.Task.run(Task.scala:131)
   	at org.apache.spark.executor.Executor$TaskRunner.$anonfun$run$3(Executor.scala:498)
   	at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:1437)
   	at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:501)
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
   	at java.lang.Thread.run(Thread.java:748)
   Caused by: java.io.IOException: Stream is corrupted
   	at net.jpountz.lz4.LZ4BlockInputStream.refill(LZ4BlockInputStream.java:200)
   	at net.jpountz.lz4.LZ4BlockInputStream.refill(LZ4BlockInputStream.java:226)
   	at net.jpountz.lz4.LZ4BlockInputStream.read(LZ4BlockInputStream.java:157)
   	at org.apache.spark.storage.BufferReleasingInputStream.read(ShuffleBlockFetcherIterator.scala:841)
   	... 25 more
   
   )
   ```
   
   
   ### Does this PR introduce any user-facing change?
   
   No
   
   ### How was this patch tested?
   
   Existing tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598297714



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       BTW, it seems to me that when io encryption enabled, the demarcation point (end of blocks) between consecutive blocks cannot be resolved correctly during deserialization (shuffle read).
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] xuanyuanking commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598454149



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       Maybe we can have this bandage fix first. The encryption shuffle mode breaks the assumption of batch serialized and deserialize, similar to the issue with codec concatenation.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #31898:
URL: https://github.com/apache/spark/pull/31898


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-804268916


   Thank you for the update and your answers, @hezuojiao !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-802850523


   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-804127836


   @dongjoon-hyun test added


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-803491706


   cc @cloud-fan, @maryannxue and @dongjoon-hyun FYI


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Ngone51 commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-803739884


   cc @xuanyuanking 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598778422



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/CoalesceShufflePartitionsSuite.scala
##########
@@ -408,6 +412,25 @@ class CoalesceShufflePartitionsSuite extends SparkFunSuite with BeforeAndAfterAl
     }
     withSparkSession(test, 100, None)
   }
+
+  test("SPARK-34790: enable IO encryption in AQE partition coalescing") {

Review comment:
       I can reproduce the issue because my laptop has only 4 cores :) 
   I updated the test case by using fixed `numInputPartitions` instead of `sparkContext.defaultParallelism` which is related to the number of computer cores.
   You can try to run this test again now. @Ngone51 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598300042



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)
 
     val doBatchFetch = shouldBatchFetch && serializerRelocatable &&
-      (!compressed || codecConcatenation) && !useOldFetchProtocol
+      (!compressed || codecConcatenation) && !useOldFetchProtocol && !ioEncryption

Review comment:
       No. In my understanding, continuous reading in batch only occurs when adaptive query execution is turned on. Furthermore, it is actually only related to partition coalescing of AQE. So it will not affect the case when the adaptive query execution is closed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598201910



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)
 
     val doBatchFetch = shouldBatchFetch && serializerRelocatable &&
-      (!compressed || codecConcatenation) && !useOldFetchProtocol
+      (!compressed || codecConcatenation) && !useOldFetchProtocol && !ioEncryption

Review comment:
       IIUC, the test coverage of this code path works correctly when `spark.sql.adaptive.enabled=false`. If not, please file another JIRA. It's independent from AQE.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-804377718


   Also, cc @mridulm , too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun edited a comment on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-804376654


   In order not to forget this, I created SPARK-34827 as a blocker for Apache Spark 3.2.0.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598300042



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)
 
     val doBatchFetch = shouldBatchFetch && serializerRelocatable &&
-      (!compressed || codecConcatenation) && !useOldFetchProtocol
+      (!compressed || codecConcatenation) && !useOldFetchProtocol && !ioEncryption

Review comment:
       No. In my understanding, continuous reading in batch only occurs when adaptive query execution is turned on. Furthermore, it is actually only related to partitions coalesced of AQE. So it will not affect the case when the adaptive query execution is closed.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-804376654


   In order not to forget this, I created SPARK-34827 `Support IO Encryption in SQL Adaptive Query Execution` as a blocker for Apache Spark 3.2.0.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598442650



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       That sounds like a good idea but we need to think about how to support it in the current shuffle protocol.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598201799



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)
 
     val doBatchFetch = shouldBatchFetch && serializerRelocatable &&
-      (!compressed || codecConcatenation) && !useOldFetchProtocol
+      (!compressed || codecConcatenation) && !useOldFetchProtocol && !ioEncryption

Review comment:
       @hezuojiao . Do you want to ban IO encryption when we don't use adaptive SQL query execution,  `spark.sql.adaptive.enabled=false`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Ngone51 commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598485027



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/CoalesceShufflePartitionsSuite.scala
##########
@@ -408,6 +412,25 @@ class CoalesceShufflePartitionsSuite extends SparkFunSuite with BeforeAndAfterAl
     }
     withSparkSession(test, 100, None)
   }
+
+  test("SPARK-34790: enable IO encryption in AQE partition coalescing") {

Review comment:
       I can't reproduce the issue by this test. @hezuojiao Could you double-check it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] mridulm commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-807235884


   Thanks for the ping @dongjoon-hyun !
   +CC @otterc to track.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31898:
URL: https://github.com/apache/spark/pull/31898#issuecomment-803800454


   This reminds me of `CompressionCodec.supportsConcatenationOfSerializedStreams`. We have to disable the shuffle batch fetch if it can't work.
   
   This patch LGTM. Later on, we can investigate how to support IO encryption with shuffle batch fetch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hezuojiao commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
hezuojiao commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598297612



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       Yes, you are right. This PR makes a simple read control, which may sacrifice shuffle read performance in AQE. It would be better to allow batch fetching for contiguous shuffle blocks while IO encryption enabled.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598200453



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       I think we still have enough time to take a look and make AQE supported with IO encryption. Does it require a lot of change to make it supported?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Ngone51 commented on a change in pull request #31898: [SPARK-34790][CORE] Disable fetching shuffle blocks in batch when io encryption is enabled

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #31898:
URL: https://github.com/apache/spark/pull/31898#discussion_r598407556



##########
File path: core/src/main/scala/org/apache/spark/shuffle/BlockStoreShuffleReader.scala
##########
@@ -51,15 +51,17 @@ private[spark] class BlockStoreShuffleReader[K, C](
       true
     }
     val useOldFetchProtocol = conf.get(config.SHUFFLE_USE_OLD_FETCH_PROTOCOL)
+    // SPARK-34790: Fetching continuous blocks in batch is incompatible with io encryption.
+    val ioEncryption = conf.get(config.IO_ENCRYPTION_ENABLED)

Review comment:
       We could store the size info of each consecutive block in `ShuffleBlockBatchId` to resolve it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org