You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dongjoon-hyun (via GitHub)" <gi...@apache.org> on 2024/01/10 22:20:53 UTC
[PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
dongjoon-hyun opened a new pull request, #44673:
URL: https://github.com/apache/spark/pull/44673
### What changes were proposed in this pull request?
This PR aims to improve `Master` to recover quickly in case of zero workers and apps.
### Why are the changes needed?
This case happens on the initial cluster creation or during restarting.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass the CIs with the newly added test case.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44673:
URL: https://github.com/apache/spark/pull/44673#issuecomment-1885870489
Thank you for review, @mridulm . The PR is updated accordingly.
--
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #44673: [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps
URL: https://github.com/apache/spark/pull/44673
--
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #44673:
URL: https://github.com/apache/spark/pull/44673#discussion_r1448067149
##########
core/src/main/scala/org/apache/spark/deploy/master/Master.scala:
##########
@@ -619,6 +619,9 @@ private[deploy] class Master(
case e: Exception => logInfo("Worker " + worker.id + " had exception on reconnect")
}
}
+
+ // In case of zero workers and apps, we can complete recovery.
+ if (canCompleteRecovery) { completeRecovery() }
}
Review Comment:
If we are completing recovery when `canCompleteRecovery == true`, we can avoid scheduling `CompleteRecovery` task (`recoveryCompletionTask`) ?
Perhaps returns a `boolean` from this method to indicate 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.
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44673:
URL: https://github.com/apache/spark/pull/44673#discussion_r1448072611
##########
core/src/main/scala/org/apache/spark/deploy/master/Master.scala:
##########
@@ -619,6 +619,9 @@ private[deploy] class Master(
case e: Exception => logInfo("Worker " + worker.id + " had exception on reconnect")
}
}
+
+ // In case of zero workers and apps, we can complete recovery.
+ if (canCompleteRecovery) { completeRecovery() }
Review Comment:
Ya, I agree. Sure, let me change.
--
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44673:
URL: https://github.com/apache/spark/pull/44673#issuecomment-1885919098
Thank you, @mridulm .
--
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
Re: [PR] [SPARK-46664][CORE] Improve `Master` to recover quickly in case of zero workers and apps [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #44673:
URL: https://github.com/apache/spark/pull/44673#discussion_r1448062743
##########
core/src/main/scala/org/apache/spark/deploy/master/Master.scala:
##########
@@ -619,6 +619,9 @@ private[deploy] class Master(
case e: Exception => logInfo("Worker " + worker.id + " had exception on reconnect")
}
}
+
+ // In case of zero workers and apps, we can complete recovery.
+ if (canCompleteRecovery) { completeRecovery() }
Review Comment:
super nit:
```suggestion
if (canCompleteRecovery) {
completeRecovery()
}
```
Not sure why this is the style in rest of the class though :-(
##########
core/src/main/scala/org/apache/spark/deploy/master/Master.scala:
##########
@@ -619,6 +619,9 @@ private[deploy] class Master(
case e: Exception => logInfo("Worker " + worker.id + " had exception on reconnect")
}
}
+
+ // In case of zero workers and apps, we can complete recovery.
+ if (canCompleteRecovery) { completeRecovery() }
}
Review Comment:
If we are completing recovery when `canCompleteRecovery == true`, we can avoid scheduling `CompleteRecovery` task ?
Perhaps returns a `boolean` from this method to indicate 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.
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