tgravescs commented on a change in pull request #30204:
URL: https://github.com/apache/spark/pull/30204#discussion_r518129798
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -114,7 +124,7 @@ private[spark] class ExecutorPodsAllocator(
// both the creation and deletion events. In either case, delete the
missing pod
Review comment:
this is called by the ExecutorPodsSnapshotsStoreImpl potentially pretty
often. Its on start, whenever you set the total executors (which would be
dynamic allocation manager updating things), and there is an allocation timer
that fires (spark.kubernetes.allocation.batch.delay=1s default) that updates
the allocator on any k8s pod changes. We only request small number of pods at a
time and iterate, so if you request 20 executors, its definitely going to go
through here a few times.
Since it can potentially be called a lot, this is why I made the normal case
with no resource profiles a fast path and constructing the resource profile to
pod state only when we are using them. I didn't actually profile that to see
how slow it could be but was thinking it still shouldn't be that bad. The
alternative is to store the resource profile to pod state but this required a
lot more changes to the snapshot store and lifecycle manager.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
