Metadata garbage collection#1563
Conversation
|
Pushed snapshot cleanup, going to add content clean up next. I think we want add a few more trigger points for GC right now (container deletion?) then we can consider this for merge. Considering this makes no changes to the underlying data structures it is safe to merge this and add a reference tracking version later. |
There was a problem hiding this comment.
Make sure to fill in the godoc for these core exports.
There was a problem hiding this comment.
This is a little odd. We shouldn't rely on this fixup on the database. Are we not able to construct the database with the correct content store?
There was a problem hiding this comment.
I was thinking that we would want to prevent overriding, however it doesn't really make sense for it to be called multiple times. I was trying to prevent changes to other parts of the code that handle this initialization, it is already somewhat complicated by the fact that the metadata db is passed around as a plugin and these functions are also initialized by plugins. The alternative to have the database object fully initialized at construction would require the metadata plugin to rely on content store and snapshot store and the services to only rely on the metadata store. That pattern may make more sense now.
There was a problem hiding this comment.
I think it is important that we ensure our initialization is worked out correctly. Usually, ergonomics problems during instantiation reflect a problem with the object model. We might be missing a component.
Let's draw a dependency diagram and work it out from there.
There was a problem hiding this comment.
Use d for the name, short for "duration". I think we used "duration` in the past.
There was a problem hiding this comment.
Let's document the locking semantics. The behavior of these is really important for implementing this safely.
There was a problem hiding this comment.
Is this m for "modification"? Would wlock be better?
There was a problem hiding this comment.
Short for "mutation", modification works as well. wlock would probably be clearer
There was a problem hiding this comment.
Alright, wlock sounds like fine. Just make sure to make it clear how these work together as a latch.
There was a problem hiding this comment.
I think you can use sub tests to do this now.
|
Removing WIP tag. I added a trigger for containerd deletion as well as well as updated content and snapshot deletion to mark their stores as dirty so the next collection will know to clean up those datastores even if the metadata collection did not mark anything for deletion. |
dmcgowan
changed the title
|
Added #1582 for metadata database changes |
|
Rebased Note that the garbage collection is relying on labels to determine the links between content. These labels are set during content creation. The first garbage collection after updating will remove content which was previously pulled. Is this an issue I should mitigate like I did with the database migration? It would still likely require some sort of repull to get the labels set on the client. Note the labeling commit is separate, it is possible we could get this commit in first if we are in agreement on the garbage collection label approach. |
Ensure all snapshots and content are referenced on commit and protected from cleanup. Signed-off-by: Derek McGowan <[email protected]>
Marks and sweeps unreferenced objects. Add snapshot cleanup to metadata. Add content garbage collection Add dirty flags for snapshotters and content store which are set on deletion and used during the next garbage collection. Cleanup content store backend when content metadata is removed. Signed-off-by: Derek McGowan <[email protected]>
Call garbage collection on container and image deletion. Signed-off-by: Derek McGowan <[email protected]>
Prevent checkpoints from getting garbage collected by adding root labels to unreferenced checkpoint objects. Mark checkpoints as gc roots. Signed-off-by: Derek McGowan <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1563 +/- ##
==========================================
+ Coverage 46.37% 48.87% +2.49%
==========================================
Files 26 27 +1
Lines 3547 4074 +527
==========================================
+ Hits 1645 1991 +346
- Misses 1524 1668 +144
- Partials 378 415 +37
Continue to review full report at Codecov.
|
|
LGTM |
1 similar comment
|
LGTM |
4 participants
Add initial implementation of a mark and sweep garbage collection on the metadata store.
Currently the garbage collection is only trigger on image deletion, this temporary.
Follows reference and root definitions defined in #1398. Labels are used to keep these references. An alternate implementation is being considered which continues to use labels but stores backreferences to allow a fully consistent delete just on the metadata store.