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 2020/12/11 08:43:07 UTC
[GitHub] [spark] cxzl25 opened a new pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
cxzl25 opened a new pull request #30725:
URL: https://github.com/apache/spark/pull/30725
### What changes were proposed in this pull request?
Modify cache(`hadoopJobMetadata`) softValues to weakValues.
### Why are the changes needed?
Reduce driver memory pressure, gc time and frequency, job execution time.
`HadoopRDD` uses soft-reference map to cache `jobconf` (rdd_id -> jobconf)
When the number of hive partitions read by the driver is large, HadoopRDD.getPartitions will create many jobconfs and add them to the cache.
The executor will also create a jobconf, add it to the cache, and share it among exeuctors.
The number of jobconfs in the driver cache increases the memory pressure. When the driver memory configuration is not high, full gc becoming very frequent, and these jobconfs are hardly reused.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Exist UT
Manual test
----------------------------------------------------------------
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] xkrogen commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-743293158
Do we really need a separate copy of the JobConf cached for each partition ID? Is there any opportunity for us to reduce the number of JobConfs to begin with? It seems like all of the partitions should be able to safely share the same conf object...?
Regardless, weak references seem more appropriate here than soft.
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-811550581
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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] cxzl25 commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-744357959
It seems that the github test is ok, and there is a performance improvement in the production environment test.
Can you review this pr if you have time ? @cloud-fan
----------------------------------------------------------------
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] cxzl25 commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-743967591
> Do we really need a separate copy of the JobConf cached for each partition ID?
Needs. HadoopRDD#getJobConf has such a comment.
https://github.com/apache/spark/blob/f8277d3aa308d267ff0423f85ffd884480cedf59/core/src/main/scala/org/apache/spark/rdd/HadoopRDD.scala#L144-L153
----------------------------------------------------------------
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 #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30725:
URL: https://github.com/apache/spark/pull/30725#discussion_r544106768
##########
File path: core/src/main/scala/org/apache/spark/SparkEnv.scala
##########
@@ -74,10 +74,10 @@ class SparkEnv (
@volatile private[spark] var isStopped = false
private val pythonWorkers = mutable.HashMap[(String, Map[String, String]), PythonWorkerFactory]()
- // A general, soft-reference map for metadata needed during HadoopRDD split computation
- // (e.g., HadoopFileRDD uses this to cache JobConfs and InputFormats).
+ // A general, weak-reference map for metadata needed during HadoopRDD split computation
+ // (e.g., HadoopRDD uses this to cache JobConfs).
private[spark] val hadoopJobMetadata =
- CacheBuilder.newBuilder().softValues().build[String, AnyRef]().asMap()
+ CacheBuilder.newBuilder().weakValues().build[String, AnyRef]().asMap()
Review comment:
is it better to put a size limitation for this cache?
----------------------------------------------------------------
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] xkrogen commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-744548273
I see, thanks for the reference. So IIUC this patch is primarily targeting the `spark.hadoop.cloneConf = true` use case?
----------------------------------------------------------------
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] xkrogen commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-746569310
> To clarify, the partition here refers to the partition of the hive table, not the rdd partition.
Now it all makes sense. Thanks for the clarification. Seems I needed to read your original message more carefully.
----------------------------------------------------------------
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] cxzl25 commented on a change in pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #30725:
URL: https://github.com/apache/spark/pull/30725#discussion_r544143219
##########
File path: core/src/main/scala/org/apache/spark/SparkEnv.scala
##########
@@ -74,10 +74,10 @@ class SparkEnv (
@volatile private[spark] var isStopped = false
private val pythonWorkers = mutable.HashMap[(String, Map[String, String]), PythonWorkerFactory]()
- // A general, soft-reference map for metadata needed during HadoopRDD split computation
- // (e.g., HadoopFileRDD uses this to cache JobConfs and InputFormats).
+ // A general, weak-reference map for metadata needed during HadoopRDD split computation
+ // (e.g., HadoopRDD uses this to cache JobConfs).
private[spark] val hadoopJobMetadata =
- CacheBuilder.newBuilder().softValues().build[String, AnyRef]().asMap()
+ CacheBuilder.newBuilder().weakValues().build[String, AnyRef]().asMap()
Review comment:
Use limited size, soft-reference cache can reduce the number of YGC than weak-reference.
But what size should it be limited to?
In fact, the driver rarely has the opportunity to reuse the jobconf of the cache, and it makes sense to share the jobconf in the executor.
----------------------------------------------------------------
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] xkrogen commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-745394031
Thanks for the further explanation, that is very helpful. Seems like potentially the comment in `HadoopRDD#getJobConf`
should be updated since the concurrency bugs have been fixed in Hadoop since 2.7.0, a pretty old version.
There is still one point I don't understand. It seems that the key for the `JobConf` in the cache is based on the ID of the RDD, not a per-partition key:
```
protected val jobConfCacheKey: String = "rdd_%d_job_conf".format(id)
```
So I would expect there to be one cached entry in `hadoopJobMetadata` per RDD. How do we end up with one `JobConf` per partition? Is it because the `check if jobConf in cache -> if not put into cache` steps are not synchronized, and many threads simultaneously decide that the conf isn't present and then put many copies of the conf into the cache? Or have I missed something?
Thanks for bearing with me as I try to understand this issue!
----------------------------------------------------------------
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] github-actions[bot] closed pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #30725:
URL: https://github.com/apache/spark/pull/30725
--
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] cxzl25 commented on a change in pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #30725:
URL: https://github.com/apache/spark/pull/30725#discussion_r540783728
##########
File path: core/src/main/scala/org/apache/spark/rdd/HadoopRDD.scala
##########
@@ -127,8 +127,6 @@ class HadoopRDD[K, V](
protected val jobConfCacheKey: String = "rdd_%d_job_conf".format(id)
- protected val inputFormatCacheKey: String = "rdd_%d_input_format".format(id)
Review comment:
SPARK-9585 Removed the inputformat cache.
----------------------------------------------------------------
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] srowen commented on a change in pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #30725:
URL: https://github.com/apache/spark/pull/30725#discussion_r546508637
##########
File path: core/src/main/scala/org/apache/spark/SparkEnv.scala
##########
@@ -74,10 +74,10 @@ class SparkEnv (
@volatile private[spark] var isStopped = false
private val pythonWorkers = mutable.HashMap[(String, Map[String, String]), PythonWorkerFactory]()
- // A general, soft-reference map for metadata needed during HadoopRDD split computation
- // (e.g., HadoopFileRDD uses this to cache JobConfs and InputFormats).
+ // A general, weak-reference map for metadata needed during HadoopRDD split computation
+ // (e.g., HadoopRDD uses this to cache JobConfs).
private[spark] val hadoopJobMetadata =
- CacheBuilder.newBuilder().softValues().build[String, AnyRef]().asMap()
+ CacheBuilder.newBuilder().weakValues().build[String, AnyRef]().asMap()
Review comment:
Whether or not a limit is in place (which could also be a good back-stop to prevent a huge cache), this could be fine - I think the only risk is that weak references are quite readily reclaimed, so this risks losing most of the caching.
----------------------------------------------------------------
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] cxzl25 commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-745878535
To clarify, the partition here refers to the partition of the hive table, not the rdd partition.
For example, using spark sql to read a hive table, the hive table has 10,000 partitions.
`HadoopTableReader#makeRDDForPartitionedTable` will create 10,000 Rdd, which means there are 10,000 jobconfs.
----------------------------------------------------------------
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 #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-743060383
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] cloud-fan commented on a change in pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30725:
URL: https://github.com/apache/spark/pull/30725#discussion_r544106768
##########
File path: core/src/main/scala/org/apache/spark/SparkEnv.scala
##########
@@ -74,10 +74,10 @@ class SparkEnv (
@volatile private[spark] var isStopped = false
private val pythonWorkers = mutable.HashMap[(String, Map[String, String]), PythonWorkerFactory]()
- // A general, soft-reference map for metadata needed during HadoopRDD split computation
- // (e.g., HadoopFileRDD uses this to cache JobConfs and InputFormats).
+ // A general, weak-reference map for metadata needed during HadoopRDD split computation
+ // (e.g., HadoopRDD uses this to cache JobConfs).
private[spark] val hadoopJobMetadata =
- CacheBuilder.newBuilder().softValues().build[String, AnyRef]().asMap()
+ CacheBuilder.newBuilder().weakValues().build[String, AnyRef]().asMap()
Review comment:
is it better to put a size limitation for this cache? then soft reference should also be fine.
----------------------------------------------------------------
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] cxzl25 commented on pull request #30725: [SPARK-33753][CORE] Reduce the memory footprint and gc of the cache (hadoopJobMetadata)
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #30725:
URL: https://github.com/apache/spark/pull/30725#issuecomment-745305483
> I see, thanks for the reference. So IIUC this patch is primarily targeting the `spark.hadoop.cloneConf = true` use case?
No.
When `spark.hadoop.cloneConf=false`, `HadoopRDD#getPartitions` will create a jobconf and add it to `hadoopJobMetadata` cache.
When the number of partitions of the queried hive table is large, many jobconf objects will be created and added to the cache.
When the drvier memory configuration is small, the driver will use all the memory, and then full gc.
If your hadoop client version is above 2.7, or use the patch of [HADOOP-11209](https://issues.apache.org/jira/browse/HADOOP-11209), you can enable `spark.hadoop.cloneConf=true`, at this time the driver will not have too many jobconf objects.
----------------------------------------------------------------
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