You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2023/06/16 10:37:52 UTC
[GitHub] [yunikorn-core] FrankYang0529 opened a new pull request, #571: [YUNIKORN-1801] add allocation events
FrankYang0529 opened a new pull request, #571:
URL: https://github.com/apache/yunikorn-core/pull/571
### What is this PR for?
Add allocation events.
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1801
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232217820
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: stopped by RM", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation'%s' in application '%s' is removed. Reason: timeout", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: preempted", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ message := fmt.Sprintf("Placeholder '%s' in application '%s' is removed. Reason: replaced", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ if event != nil {
+ evt.eventSystem.AddEvent(event)
+ }
+}
+
+func (evt *applicationEvents) sendTerminateAllocationAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation ask '%s' in application '%s' is removed. Reason: timeout", request.GetAllocationKey(), request.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_REMOVE, si.EventRecord_REQUEST_TIMEOUT, request.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation ask '%s' in application '%s' is removed. Reason: cancel", request.GetAllocationKey(), request.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_REMOVE, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ }
+
+ if event != nil {
Review Comment:
Make sure we always have a valid event so we don't need this nil check 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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233850857
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
Yeah, I see. The application removal event is in ticket 1800.
<img width="643" alt="截圖 2023-06-19 下午6 16 45" src="https://github.com/apache/yunikorn-core/assets/4927361/d865b227-d3da-4326-b39f-3b836ad290bc">
But there is another event for asks which is trigger by application removal.
<img width="642" alt="截圖 2023-06-19 下午6 17 37" src="https://github.com/apache/yunikorn-core/assets/4927361/6be866c7-1997-43e6-abb7-79cf38ba5ea0">
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233776724
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
It looks like we don't have a termination type distinguish between "application removal" and "request removal". Currently, I use `TerminationType_STOPPED_BY_RM` for "request removal". Can I use `TerminationType_UNKNOWN_TERMINATION_TYPE` for "application removal"?
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wilfred-s commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "wilfred-s (via GitHub)" <gi...@apache.org>.
wilfred-s commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1602649176
> Not sure why CodeCov shows it's not covered.
The tests are not in the objects package that skews the details in the codecov sometimes as the existing test coverage is not counted.
Change looks good.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232228904
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
Review Comment:
`sendNewAskEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1598516229
@FrankYang0529 thanks for the update, the patch now is in very good shape. There are some minor complaints from codecov, please check those, new lines should be covered.
I'll let Wilfred/Mani take a look as well.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232221701
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: stopped by RM", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation'%s' in application '%s' is removed. Reason: timeout", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: preempted", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ message := fmt.Sprintf("Placeholder '%s' in application '%s' is removed. Reason: replaced", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ if event != nil {
Review Comment:
Make sure we always have a valid event so we don't need this nil check 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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1235060594
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_TIMEOUT:
Review Comment:
Updated it. Thanks.
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
Review Comment:
Updated it. Thanks.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232516214
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2017,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) SendTerminateAllocationAskEvent(allocKey string) {
Review Comment:
> It's not a good idea to check whether allocKey is empty in Application.RemoveAllocationAsk and sent related events
Why? An empty string means that we cancel all asks. All we have to do is iterate through `sa.requests` and send the related events. If we have 100 asks, then so be it.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232220872
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
Review Comment:
Just `sendAllocationEvent()
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
Review Comment:
Just `sendAllocationEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1235042063
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2018,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) sendRemoveAskEvent(allocKey string) {
+ sa.RLock()
+ defer sa.RUnlock()
Review Comment:
Removed it. Thanks!
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
Updated it. Thanks!
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233710437
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2018,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) sendRemoveAskEvent(allocKey string) {
+ sa.RLock()
+ defer sa.RUnlock()
Review Comment:
This lock must be eliminated because the method is called from a locked call site. If it's needed then you have to create another one without locks.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1596739876
@FrankYang0529 another important thing is to update the tests:
1. Update `events_test.go` based on existing test cases.
2. Update `application_test.go` to see that new events are indeed properly generated.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233898044
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
Yes, we call `removeAsksInternal("")` in two places. One is the timeout case (placeholders), the other is when we remove the entire application. You know the context, just pass this extra info to the `sendRemoveAskEvent()` method.
Eg. a bool flag like `appRemoved`, if it's false, then it's a timeout.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1598924352
> There are some minor complaints from codecov, please check those, new lines should be covered.
* Line `pkg/scheduler/objects/application.go#L1150` is covered by `TestPlaceholderMatch`.
* Line `pkg/scheduler/objects/application.go#L1206` is covered by `TestFailReplacePlaceholder`.
Not sure why CodeCov shows it's not covered.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1599121820
We can get the correct coverage result if we add `-coverpkg` to the test command like:
```
go test ./... -run TestFailReplacePlaceholder -tags deadlock -coverprofile=coverage.txt -covermode=atomic -coverpkg=`go list ./pkg/... | paste -sd "," -`
```
@pbacsko, Do we want to update it in `Makefile`? Thanks
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233387895
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_TIMEOUT:
Review Comment:
Same here, just set a variable.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] codecov[bot] commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1598497370
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-core/pull/571?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#571](https://app.codecov.io/gh/apache/yunikorn-core/pull/571?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (87ba731) into [master](https://app.codecov.io/gh/apache/yunikorn-core/commit/a6a90bb0408a3004592f06d8a79335ac326dcfe4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (a6a90bb) will **increase** coverage by `0.12%`.
> The diff coverage is `97.75%`.
```diff
@@ Coverage Diff @@
## master #571 +/- ##
==========================================
+ Coverage 75.45% 75.58% +0.12%
==========================================
Files 73 73
Lines 12057 12126 +69
==========================================
+ Hits 9098 9165 +67
- Misses 2638 2640 +2
Partials 321 321
```
| [Impacted Files](https://app.codecov.io/gh/apache/yunikorn-core/pull/571?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/application.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/571?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `65.82% <91.30%> (+0.41%)` | :arrow_up: |
| [pkg/scheduler/objects/application\_events.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/571?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232439850
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2017,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) SendTerminateAllocationAskEvent(allocKey string) {
Review Comment:
Canceling AllocationAsk cause of removing an application is in `PartitionContext.removeApplication`. It calls `app.RemoveAllocationAsk("")`.
https://github.com/apache/yunikorn-core/blob/a6a90bb0408a3004592f06d8a79335ac326dcfe4/pkg/scheduler/partition.go#L375-L382
Removing AllocationAsk cause of releasing AllocationAsk request is in `PartitionContext.removeAllocation`. It calls `app.RemoveAllocationAsk(allocKey)`.
https://github.com/apache/yunikorn-core/blob/a6a90bb0408a3004592f06d8a79335ac326dcfe4/pkg/scheduler/partition.go#L1347-L1369
It's not a good idea to check whether `allocKey` is empty in `Application.RemoveAllocationAsk` and sent related events, so I export the function for `PartitionContext` to use. I thought about giving another argument like [TerminationType](https://github.com/apache/yunikorn-scheduler-interface/blob/12c630245d861e48c66841a05117bfc55c00506b/lib/go/si/si.pb.go#L30-L34), but we can't tell if the application was removed or the release AllocationAsk was sent by the shim.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1594478322
Hi @pbacsko, I am not 100% sure that I added events to correct places. May you help me review it first? If all allocation events are fine , I will add test cases. Thanks!
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232221358
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
Review Comment:
`sendRemoveAllocationEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232219894
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2017,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) SendTerminateAllocationAskEvent(allocKey string) {
Review Comment:
Do we need to export this?
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233388000
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
It's ok to have a single method for remove/cancel, just merge the two.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233387800
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
Review Comment:
This can be greatly simplified: everything is the same except the change detail. Just set a single variable based on `terminationType`.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232217352
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
Review Comment:
Message is not needed, we have event details, that should suffice.
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: stopped by RM", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation'%s' in application '%s' is removed. Reason: timeout", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: preempted", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ message := fmt.Sprintf("Placeholder '%s' in application '%s' is removed. Reason: replaced", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ if event != nil {
+ evt.eventSystem.AddEvent(event)
+ }
+}
+
+func (evt *applicationEvents) sendTerminateAllocationAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation ask '%s' in application '%s' is removed. Reason: timeout", request.GetAllocationKey(), request.GetApplicationID())
Review Comment:
Message is not needed.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232220872
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
Review Comment:
Just `sendNewAllocationEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233782576
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2017,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) SendTerminateAllocationAskEvent(allocKey string) {
Review Comment:
In this case, we assume that:
* If `app.RemoveAllocationAsk("")` is called, it's from application removal.
* If `app.RemoveAllocationAsk(allocKey)` is called, it's from request removal.
Not sure whether we will have other termination type calls `app.RemoveAllocationAsk` in the future. Currently, we only have these two possibilities, so I think it's okay to have the assumption.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233780085
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
You shouldn't be concerned with application removal, that's a different JIRA ticket. This task is only about allocations/asks.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] FrankYang0529 commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232439850
##########
pkg/scheduler/objects/application.go:
##########
@@ -2009,3 +2017,20 @@ func (sa *Application) SetTimedOutPlaceholder(taskGroupName string, timedOut int
sa.placeholderData[taskGroupName].TimedOut = timedOut
}
}
+
+func (sa *Application) SendTerminateAllocationAskEvent(allocKey string) {
Review Comment:
Canceling AllocationAsk cause of removing an application is in `PartitionContext.removeApplication`. It calls `app. RemoveAllocationAsk("")`.
https://github.com/apache/yunikorn-core/blob/a6a90bb0408a3004592f06d8a79335ac326dcfe4/pkg/scheduler/partition.go#L375-L382
Removing AllocationAsk cause of releasing AllocationAsk request is in `PartitionContext.removeAllocation`. It calls `app.RemoveAllocationAsk(allocKey)`.
https://github.com/apache/yunikorn-core/blob/a6a90bb0408a3004592f06d8a79335ac326dcfe4/pkg/scheduler/partition.go#L1347-L1369
It's not a good idea to check whether `allocKey` is empty in `Application.RemoveAllocationAsk` and sent related events, so I export the function for `PartitionContext` to use. I thought about giving another argument like [TerminationType](https://github.com/apache/yunikorn-scheduler-interface/blob/12c630245d861e48c66841a05117bfc55c00506b/lib/go/si/si.pb.go#L30-L34), but we can't tell if the application was removed or the release AllocationAsk was sent by the shim.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232224079
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: stopped by RM", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation'%s' in application '%s' is removed. Reason: timeout", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: preempted", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ message := fmt.Sprintf("Placeholder '%s' in application '%s' is removed. Reason: replaced", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ if event != nil {
+ evt.eventSystem.AddEvent(event)
+ }
+}
+
+func (evt *applicationEvents) sendTerminateAllocationAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation ask '%s' in application '%s' is removed. Reason: timeout", request.GetAllocationKey(), request.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_REMOVE, si.EventRecord_REQUEST_TIMEOUT, request.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation ask '%s' in application '%s' is removed. Reason: cancel", request.GetAllocationKey(), request.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_REMOVE, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ }
+
+ if event != nil {
+ evt.eventSystem.AddEvent(event)
+ }
+}
+
+func (evt *applicationEvents) sendRemoveApplicationTerminateAllocationAskEvent(request *AllocationAsk) {
Review Comment:
`sendCancelAskEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1232223222
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,87 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendSuccessfulAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Allocation '%s' in application '%s' is successfully allocated to node '%s'", alloc.GetUUID(), alloc.GetApplicationID(), alloc.GetNodeID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendIncomingAllocationAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ message := fmt.Sprintf("Incoming allocation ask '%s' for application '%s'", request.GetAllocationKey(), request.GetApplicationID())
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, message, request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendTerminateAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var message string
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: node is removed", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: stopped by RM", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ message = fmt.Sprintf("Allocation'%s' in application '%s' is removed. Reason: timeout", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ message = fmt.Sprintf("Allocation '%s' in application '%s' is removed. Reason: preempted", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ message := fmt.Sprintf("Placeholder '%s' in application '%s' is removed. Reason: replaced", alloc.GetUUID(), alloc.GetApplicationID())
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, message, alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ if event != nil {
+ evt.eventSystem.AddEvent(event)
+ }
+}
+
+func (evt *applicationEvents) sendTerminateAllocationAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
`sendRemoveAskEvent()`
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#discussion_r1233898044
##########
pkg/scheduler/objects/events.go:
##########
@@ -50,6 +51,71 @@ func (evt *applicationEvents) sendPlaceholderLargerEvent(ph *Allocation, request
evt.eventSystem.AddEvent(event)
}
+func (evt *applicationEvents) sendNewAllocationEvent(alloc *Allocation) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_ADD, si.EventRecord_APP_ALLOC, alloc.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendNewAskEvent(request *AllocationAsk) {
+ if !evt.enabled {
+ return
+ }
+
+ event := events.CreateAppEventRecord(evt.app.ApplicationID, "", request.GetAllocationKey(), si.EventRecord_ADD, si.EventRecord_APP_REQUEST, request.GetAllocatedResource())
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAllocationEvent(alloc *Allocation, terminationType si.TerminationType) {
+ if !evt.enabled {
+ return
+ }
+
+ var event *si.EventRecord
+ switch terminationType {
+ case si.TerminationType_UNKNOWN_TERMINATION_TYPE:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_NODEREMOVED, alloc.GetAllocatedResource())
+ case si.TerminationType_STOPPED_BY_RM:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_CANCEL, alloc.GetAllocatedResource())
+ case si.TerminationType_TIMEOUT:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_TIMEOUT, alloc.GetAllocatedResource())
+ case si.TerminationType_PREEMPTED_BY_SCHEDULER:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_PREEMPT, alloc.GetAllocatedResource())
+ case si.TerminationType_PLACEHOLDER_REPLACED:
+ event = events.CreateAppEventRecord(evt.app.ApplicationID, "", alloc.GetUUID(), si.EventRecord_REMOVE, si.EventRecord_ALLOC_REPLACED, alloc.GetAllocatedResource())
+ }
+
+ evt.eventSystem.AddEvent(event)
+}
+
+func (evt *applicationEvents) sendRemoveAskEvent(request *AllocationAsk, terminationType si.TerminationType) {
Review Comment:
Yes, we call `removeAsksInternal("")` in two places. One is the timeout case (placeholders), the other is when we remove the entire application. You know the context, just pass this extra info to the `sendRemoveAskEvent()` method.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko closed pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko closed pull request #571: [YUNIKORN-1801] add allocation events
URL: https://github.com/apache/yunikorn-core/pull/571
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on pull request #571: [YUNIKORN-1801] add allocation events
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on PR #571:
URL: https://github.com/apache/yunikorn-core/pull/571#issuecomment-1600293707
> We can get the correct coverage result if we add `-coverpkg` to the test command like:
>
> ```
> go test ./... -run TestFailReplacePlaceholder -tags deadlock -coverprofile=coverage.txt -covermode=atomic -coverpkg=`go list ./pkg/... | paste -sd "," -`
> ```
>
> @pbacsko, Do we want to update it in `Makefile`? Thanks
I'll take a note. If everything passes and is OK besides this, we're good to go.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org