Adding LocalScheduler scaling impl with UpdateTopologyManager #1333
Conversation
|
@kramasamy I can review it partially. I have authored a part of this commit (billonahill#2) which @billonahill had reviewed earlier. |
| stateManager.setPackingPlan(proposedProtoPackingPlan, topologyName)); | ||
|
|
||
| // delete the physical plan so TMaster doesn't try to re-establish it on start-up. | ||
| logFine("Deleted Physical Plan: %s", stateManager.deletePhysicalPlan(topologyName)); |
There was a problem hiding this comment.
In the past we discussed about issues when multiple components were updating a PackingPlan (Launcher while submitting a topology and Scheduler while updating the topology). In this case, PhysicalPlan is also getting updated my multiple components; RuntimeManager during kill, TMaster during submit and UpdateManager during update. Is it a concern?
There was a problem hiding this comment.
Typically the launcher (or the TMaster in the case of physical plan) initially writes object and RuntimeManager deletes them on kill. That pattern is somewhat standard.
Where we're deviating is with the Scheduler updating PackingPlan and Topology and deleting PhysicalPlan (TMaster will re-create it). I think this is ok for this iteration, but we might want to consider messaging the TMaster to do these operations as a future enhancement.
|
@billonahill Looks good to me. Maybe we should add a comment at ContainerDelta to clarify that the code assumes that the size of an existing container cannot change from currentPlan to proposedPlan. |
* Update based on master changes to remove Resource and change containerId to int * Change ContainerDelta logic to be based on container id, not container equality * fix bad javadoc html * Remove check that existing packing plan hasn't changed * Added comments to ContainerDelta about not supporting container size changes
Now that
IScalableis committed, committing theLocalSchedulerimplementation, along withUpdateTopologyManager, which is to be shared among all scheduler implementations. The scheduler changes are minimal with most of the logic contained inUpdateTopologyManager.Required for #1292.