[AMORO-2714]Delete resource when optimizer instance expired #2715
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2715 +/- ##
============================================
- Coverage 34.02% 34.01% -0.02%
- Complexity 4363 4364 +1
============================================
Files 604 604
Lines 50752 50753 +1
Branches 6673 6673
============================================
- Hits 17267 17262 -5
- Misses 32085 32093 +8
+ Partials 1400 1398 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -548,6 +548,7 @@ public void run() { | |||
| .forEach(task -> retryTask(task, queue))); | |||
| if (isExpired) { | |||
| LOG.info("Optimizer {} has been expired, unregister it", keepingTask.getOptimizer()); | |||
| deleteResource(keepingTask.getOptimizer().getResourceId()); | |||
There was a problem hiding this comment.
The createResource() maybe called by user.
Is it proper that system auto deleting resources managed by user himself?
There was a problem hiding this comment.
the createResource is actually called by user,
and the deleteResource is also will be called by user, if user want the release the optimzier,
but if the optimizer instance failed by some unexpected casuse, the system will delete the instance auto, but will
not clean the resource record in the record, and this resource will never be used again and will cause dead dirty data in the db forever.
so when the instance is expired and be deleted by system, the resource associate with this instance should also be cleand
There was a problem hiding this comment.
I don't quite agree with that.
The record left in db has its meaning, cause when the optimizer came back, AMS could identify this resource.
What you said I think is a dashboard design issue, the optimizer page don't reveal inactive optimizers. I think we could improve in that way.
A good and simple princeple is who creates who deletes.
There was a problem hiding this comment.
thanks your comment jin, agree with it, and will check the lifetime of resource again ,and get one graceful idea the resolve it.
|
@Yao-MR Thanks a lot for driving this improvement. I have some questions about this PR:
|
|
consider the pr do not resolve this issue in an graceful way, will review this issue again, and redesign the code |
Why are the changes needed?
Close #2714
Brief change log
Delete resource when optimizer instance expired which will cause dirty data in db. and this data will never be used again.
How was this patch tested?
Documentation