wangdatan commented on a change in pull request #185:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/185#discussion_r455939139
##########
File path: pkg/scheduler/scheduling_application.go
##########
@@ -325,39 +328,50 @@ func (sa *SchedulingApplication) reserve(node
*SchedulingNode, ask *schedulingAl
// unReserve the application for this node and ask combination.
// This first removes the reservation from the node.
-// The error is set if the reservation key cannot be generated on the app or
node.
-// If the reservation does not exist it returns false, if the reservation is
removed it returns true.
-func (sa *SchedulingApplication) unReserve(node *SchedulingNode, ask
*schedulingAllocationAsk) error {
+// If the reservation does not exist it returns 0 for reservations removed, if
the reservation is removed it returns 1.
+// The error is set if the reservation key cannot be removed from the app or
node.
+func (sa *SchedulingApplication) unReserve(node *SchedulingNode, ask
*schedulingAllocationAsk) (int, error) {
sa.Lock()
defer sa.Unlock()
return sa.unReserveInternal(node, ask)
}
// Unlocked version for unReserve that really does the work.
// Must only be called while holding the application lock.
-func (sa *SchedulingApplication) unReserveInternal(node *SchedulingNode, ask
*schedulingAllocationAsk) error {
+func (sa *SchedulingApplication) unReserveInternal(node *SchedulingNode, ask
*schedulingAllocationAsk) (int, error) {
resKey := reservationKey(node, nil, ask)
if resKey == "" {
log.Logger().Debug("unreserve reservation key create failed
unexpectedly",
zap.String("appID", sa.ApplicationInfo.ApplicationID),
zap.Any("node", node),
zap.Any("ask", ask))
- return fmt.Errorf("reservation key failed node or ask are nil
for appID %s", sa.ApplicationInfo.ApplicationID)
+ return 0, fmt.Errorf("reservation key failed node or ask are
nil for appID %s", sa.ApplicationInfo.ApplicationID)
}
- // find the reservation and then unReserve the node before removing
from the app
+ // unReserve the node before removing from the app
+ var num int
+ var err error
+ if num, err = node.unReserve(sa, ask); err != nil {
+ return 0, err
+ }
+ // if the unreserve worked on the node check the app
if _, found := sa.reservations[resKey]; found {
- if err := node.unReserve(sa, ask); err != nil {
- return err
+ // worked on the node means either found or not but no error,
log difference here
+ if num == 0 {
+ log.Logger().Debug("reservation not found while
removing from node, app has reservation",
Review comment:
I think we should use Info here.
##########
File path: pkg/scheduler/scheduling_application.go
##########
@@ -325,39 +328,50 @@ func (sa *SchedulingApplication) reserve(node
*SchedulingNode, ask *schedulingAl
// unReserve the application for this node and ask combination.
// This first removes the reservation from the node.
-// The error is set if the reservation key cannot be generated on the app or
node.
-// If the reservation does not exist it returns false, if the reservation is
removed it returns true.
-func (sa *SchedulingApplication) unReserve(node *SchedulingNode, ask
*schedulingAllocationAsk) error {
+// If the reservation does not exist it returns 0 for reservations removed, if
the reservation is removed it returns 1.
+// The error is set if the reservation key cannot be removed from the app or
node.
+func (sa *SchedulingApplication) unReserve(node *SchedulingNode, ask
*schedulingAllocationAsk) (int, error) {
sa.Lock()
defer sa.Unlock()
return sa.unReserveInternal(node, ask)
}
// Unlocked version for unReserve that really does the work.
// Must only be called while holding the application lock.
-func (sa *SchedulingApplication) unReserveInternal(node *SchedulingNode, ask
*schedulingAllocationAsk) error {
+func (sa *SchedulingApplication) unReserveInternal(node *SchedulingNode, ask
*schedulingAllocationAsk) (int, error) {
resKey := reservationKey(node, nil, ask)
if resKey == "" {
log.Logger().Debug("unreserve reservation key create failed
unexpectedly",
zap.String("appID", sa.ApplicationInfo.ApplicationID),
zap.Any("node", node),
zap.Any("ask", ask))
- return fmt.Errorf("reservation key failed node or ask are nil
for appID %s", sa.ApplicationInfo.ApplicationID)
+ return 0, fmt.Errorf("reservation key failed node or ask are
nil for appID %s", sa.ApplicationInfo.ApplicationID)
}
- // find the reservation and then unReserve the node before removing
from the app
+ // unReserve the node before removing from the app
+ var num int
+ var err error
+ if num, err = node.unReserve(sa, ask); err != nil {
+ return 0, err
+ }
+ // if the unreserve worked on the node check the app
if _, found := sa.reservations[resKey]; found {
- if err := node.unReserve(sa, ask); err != nil {
- return err
+ // worked on the node means either found or not but no error,
log difference here
+ if num == 0 {
+ log.Logger().Debug("reservation not found while
removing from node, app has reservation",
+ zap.String("appID",
sa.ApplicationInfo.ApplicationID),
+ zap.String("nodeID", node.NodeID),
+ zap.String("ask", ask.AskProto.AllocationKey))
}
delete(sa.reservations, resKey)
- return nil
+ return 1, nil
}
// reservation was not found
log.Logger().Debug("reservation not found while removing from app",
Review comment:
same comment, suggest to use info here.
----------------------------------------------------------------
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]
