Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17882#discussion_r122336025
--- Diff:
resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
---
@@ -68,6 +68,12 @@ private[spark] abstract class YarnSchedulerBackend(
// Flag to specify whether this schedulerBackend should be reset.
private var shouldResetOnAmRegister = false
+ private val currentState = new CurrentAMState(0,
+ RequestExecutors(Utils.getDynamicAllocationInitialExecutors(conf), 0,
Map.empty, Set.empty))
+
+ protected class CurrentAMState(
--- End diff --
The way this class is used isn't really helping. You could have the two
fields as mutable fields in the parent instead of declaring a separate type.
I'd suggest either getting rid of this class, or turning it into a proper
type to be sent as a reply to the "get" RPC, defined in
`CoarseGrainedClusterMessage.scala` (meaning you'd keep state in fields here,
and the message itself would be instantiated only when replying to the RPC, and
would be immutable).
Also, either suggestion means you don't need the changes around
`setCurrentExecutorIdCounter`. The old code was fine.
Finally, I know I suggested the name, but making the names of the classes
more generic would be better (since that file is not YARN-specific). e.g.
`GetExecutorAllocatorState` and `ExecutorAllocatorState` for the reply.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
