wilfred-s commented on a change in pull request #56:
URL:
https://github.com/apache/incubator-yunikorn-scheduler-interface/pull/56#discussion_r738021448
##########
File path: scheduler-interface-spec.md
##########
@@ -563,54 +598,50 @@ State transition of node:
See protocol below:
-Registration of a new node with the scheduler. If the node exists then the
request will be rejected.
-```protobuf
-message NewNodeInfo {
- // ID of node, must be unique
- string nodeID = 1;
- // node attributes
- map<string, string> attributes = 2;
- // Schedulable Resource
- Resource schedulableResource = 3;
- // Occupied Resource
- Resource occupiedResource = 4;
- // Allocated resources, this will be added when node registered to RM
(recovery)
- repeated Allocation existingAllocations = 5;
-}
-```
-
-Update of a registered node with the scheduler. If the node does not exist the
update will fail.
+During new node registration with the scheduler, request will be rejected if
the node exist already.
+While updating registered node with the scheduler, request will fail if the
node doesn't exist.
```protobuf
-message UpdateNodeInfo {
+message NodeInfo {
// Action from RM
enum ActionFromRM {
+
+ // Create Node
+ CREATE = 0;
Review comment:
For ENUM in protobuf the recommendation is to use 0 as UNSET and then
add the next values (1 and higher) as the real values. The 0 value in most
languages is the default for an integer and it is really difficult to
distinguish a missing value from a real value.
This is a good time to do this
##########
File path: scheduler-interface-spec.md
##########
@@ -201,78 +245,69 @@ message RegisterResourceManagerResponse {
Below is overview of how scheduler/RM keep connection and updates.
```protobuf
-message UpdateRequest {
+message AllocationRequest {
// New allocation requests or replace existing allocation request (if
allocationID is same)
repeated AllocationAsk asks = 1;
// Allocations can be released.
AllocationReleasesRequest releases = 2;
-
- // New node can be scheduled. If a node is notified to be "unscheduable", it
needs to be part of this field as well.
- repeated NewNodeInfo newSchedulableNodes = 3;
-
- // Update nodes for existing schedulable nodes.
- // May include:
- // - Node resource changes. (Like grows/shrinks node resource)
- // - Node attribute changes. (Including node-partition concept like YARN,
and concept like "local images".
- //
- // Should not include:
- // - Allocation-related changes with the node.
- // - Realtime Utilizations.
- repeated UpdateNodeInfo updatedNodes = 4;
-
+
// ID of RM, this will be used to identify which RM of the request comes
from.
- string rmID = 5;
+ string rmID = 3;
+}
+message ApplicationRequest {
// RM should explicitly add application when allocation request also
explictly belongs to application.
// This is optional if allocation request doesn't belong to a application.
(Independent allocation)
- repeated AddApplicationRequest newApplications = 6;
+ repeated AddApplicationRequest new = 1;
// RM can also remove applications, all allocation/allocation requests
associated with the application will be removed
- repeated RemoveApplicationRequest removeApplications = 7;
+ repeated RemoveApplicationRequest remove = 2;
+
+ // ID of RM, this will be used to identify which RM of the request comes
from.
+ string rmID = 3;
}
-message UpdateResponse {
- // Scheduler can send action to RM.
- enum ActionFromScheduler {
- // Nothing needs to do
- NOACTION = 0;
-
- // Something is wrong, RM needs to stop the RM, and re-register with
scheduler.
- RESYNC = 1;
- }
+message NodeRequest {
+ // New node can be scheduled. If a node is notified to be "unscheduable", it
needs to be part of this field as well.
+ repeated NodeInfo nodes = 1;
- // What RM needs to do, scheduler can send control code to RM when something
goes wrong.
- // Don't use/expand this field for other general purposed actions. (Like
kill a remote container process).
- ActionFromScheduler action = 1;
+ // ID of RM, this will be used to identify which RM of the request comes
from.
+ string rmID = 2;
+}
+message AllocationResponse {
// New allocations
- repeated Allocation newAllocations = 2;
+ repeated Allocation new = 11;
Review comment:
This looks like a typo, should be 1 not 11
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
