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 2022/02/18 05:19:55 UTC
[GitHub] [spark] LuciferYang opened a new pull request #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
LuciferYang opened a new pull request #35566:
URL: https://github.com/apache/spark/pull/35566
### What changes were proposed in this pull request?
This pr aims to cleanup unused ยท`private methods/fields`.
### Why are the changes needed?
Code clean.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass GA
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809719155
##########
File path: core/src/main/scala/org/apache/spark/rdd/NewHadoopRDD.scala
##########
@@ -244,7 +244,6 @@ class NewHadoopRDD[K, V](
}
private var havePair = false
- private var recordsSinceMetricsUpdate = 0
Review comment:
SPARK-2621 add `recordsSinceMetricsUpdate` for task metrics and after SPARK-4092 it never used
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35566:
URL: https://github.com/apache/spark/pull/35566#issuecomment-1046043576
thanks
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809727165
##########
File path: graphx/src/main/scala/org/apache/spark/graphx/impl/GraphImpl.scala
##########
@@ -265,14 +264,6 @@ class GraphImpl[VD: ClassTag, ED: ClassTag] protected (
}
}
- /** Test whether the closure accesses the attribute with name `attrName`. */
- private def accessesVertexAttr(closure: AnyRef, attrName: String): Boolean = {
Review comment:
[Handle ClassNotFoundException from ByteCodeUtils](https://github.com/apache/spark/commit/bee1015620de28806660f20d7c9f404dd3bdb2cb) introduce this method and it used by `mapReduceTriplets`, [Code cleaning to improve readability.](https://github.com/apache/spark/commit/d58bfa85738adb57a5311f52f52ede5903a181e1) clean up `mapReduceTriplets` from `GraphImpl.scala`, `mapReduceTriplets` is no longer used
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809709659
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -95,11 +93,6 @@ private[deploy] class Master(
// After onStart, webUi will be set
private var webUi: MasterWebUI = null
- private val masterPublicAddress = {
Review comment:
`masterPublicAddress` add in [Use external addresses in standalone WebUI on EC2.](https://github.com/apache/spark/commit/cdaa0fad51c7ad6c2a56f6c14faedd08fe341b2e) and never used after [SPARK-33774](https://issues.apache.org/jira/browse/SPARK-33774)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809710946
##########
File path: core/src/main/scala/org/apache/spark/deploy/rest/SubmitRestProtocolMessage.scala
##########
@@ -46,9 +46,6 @@ private[rest] abstract class SubmitRestProtocolMessage {
val action: String = messageType
var message: String = null
- // For JSON deserialization
- private def setAction(a: String): Unit = { }
Review comment:
[SPARK-5388](https://issues.apache.org/jira/browse/SPARK-5388) add this field but it never seems to be called
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809715204
##########
File path: core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala
##########
@@ -65,10 +64,6 @@ private[spark] class CoarseGrainedExecutorBackend(
var executor: Executor = null
@volatile var driver: Option[RpcEndpointRef] = None
- // If this CoarseGrainedExecutorBackend is changed to support multiple threads, then this may need
- // to be changed so that we don't share the serializer instance across threads
- private[this] val ser: SerializerInstance = env.closureSerializer.newInstance()
Review comment:
[SPARK-3386](https://issues.apache.org/jira/browse/SPARK-3386) move `ser` out of `receive` method, it used for deserialize of `TaskDescription`, but SPARK-17931 change to use `TaskDescription` do this work
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809695606
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -53,8 +53,6 @@ private[deploy] class Master(
private val forwardMessageThread =
ThreadUtils.newDaemonSingleThreadScheduledExecutor("master-forward-message-thread")
- private val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
Review comment:
SPARK-2261 `hadoopConf` and used by `rebuildSparkUI` in the pass, SPARK-12299 Remove history serving functionality from Master and `hadoopConf` become unused
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -53,8 +53,6 @@ private[deploy] class Master(
private val forwardMessageThread =
ThreadUtils.newDaemonSingleThreadScheduledExecutor("master-forward-message-thread")
- private val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
Review comment:
SPARK-2261 add `hadoopConf` and used by `rebuildSparkUI` in the pass, SPARK-12299 Remove history serving functionality from Master and `hadoopConf` become unused
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 closed pull request #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #35566:
URL: https://github.com/apache/spark/pull/35566
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809721635
##########
File path: core/src/main/scala/org/apache/spark/resource/ResourceUtils.scala
##########
@@ -385,8 +385,6 @@ private[spark] object ResourceUtils extends Logging {
val pluginClasses = sparkConf.get(RESOURCES_DISCOVERY_PLUGIN) :+ discoveryScriptPlugin
val resourcePlugins = Utils.loadExtensions(classOf[ResourceDiscoveryPlugin], pluginClasses,
sparkConf)
- // apply each plugin until one of them returns the information for this resource
- var riOption: Optional[ResourceInformation] = Optional.empty()
Review comment:
SPARK-30689 add `riOption` but it was not used because there is a definition with the same name in `resourcePlugins.foreach` loop
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #35566: [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809695606
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -53,8 +53,6 @@ private[deploy] class Master(
private val forwardMessageThread =
ThreadUtils.newDaemonSingleThreadScheduledExecutor("master-forward-message-thread")
- private val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
Review comment:
SPARK-4011 add `hadoopConf` and used by `rebuildSparkUI`, SPARK-12299 Remove history serving functionality from Master and `hadoopConf` become unused
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 pull request #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35566:
URL: https://github.com/apache/spark/pull/35566#issuecomment-1046038008
Merged to master
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #35566:
URL: https://github.com/apache/spark/pull/35566#discussion_r809971732
##########
File path: core/src/main/scala/org/apache/spark/deploy/rest/SubmitRestProtocolMessage.scala
##########
@@ -46,9 +46,6 @@ private[rest] abstract class SubmitRestProtocolMessage {
val action: String = messageType
var message: String = null
- // For JSON deserialization
- private def setAction(a: String): Unit = { }
Review comment:
I'm worried that the JSON deserializer needs this, per comment
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #35566: [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35566:
URL: https://github.com/apache/spark/pull/35566#issuecomment-1044496146
> I'm worried that the JSON deserializer needs this, per comment
Ok, [b2f2f35](https://github.com/apache/spark/pull/35566/commits/b2f2f35973dfcdb736f943d72b1756f7c99352db) revert change of `SubmitRestProtocolMessage`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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