-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YUNIKORN-459]Implement the placeholder cleanup in PlaceholderManager #207
Conversation
Codecov Report
@@ Coverage Diff @@
## YUNIKORN-2 #207 +/- ##
=============================================
Coverage ? 59.23%
=============================================
Files ? 36
Lines ? 3120
Branches ? 0
=============================================
Hits ? 1848
Misses ? 1192
Partials ? 80 Continue to review full report at Codecov.
|
pkg/cache/placeholder_manager.go
Outdated
err = app.removeTask(taskID) | ||
if err != nil { | ||
log.Logger().Error("failed to remove task from application", | ||
zap.Error(err)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
second thought, let's remove this. We do not need to remove the task from the app while cleaning up the placeholders. Task needs to be cleaned up separately, the normal process involves the core side to release corresponding resources in the scheduler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Clean up all placeholder pod and task.
Please review @yangwwei
Thanks.