-
Notifications
You must be signed in to change notification settings - Fork 886
rkt: implement initial image gc for the tree store. #1487
Conversation
I'm not sure if all the above needed different kind of store Edit: For this reason I waited to add a systemd service or change the current one to also call |
af9a9ab
to
7884fa1
Compare
763f1d7
to
f9a6ca7
Compare
Addressed by this PR.
This can be done in a follow-up PR.
This seems like some sort of
I'm hesitant whether to run I think |
return fmt.Errorf("error removing the tree store: %v", err) | ||
} | ||
return nil | ||
} | ||
|
||
func (ds Store) GetTreeStoreIDs() ([]string, error) { |
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.
docs?
f9a6ca7
to
ae025e9
Compare
Agree. Removed them from the commit message.
Agree |
Please rebase |
ae025e9
to
44f564b
Compare
} | ||
|
||
if err := gcTreeStore(s); err != nil { | ||
stderr("rkt: failed to remove unreferenced treeStores: %v", 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.
Sometimes we print treeStore(s) and sometimes treestore(s). I don't have a preference but we should be consistent :)
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.
Made all the printed one lowercase (except ID
).
My bad but I'm also not sure if it's better to say (given that a treestore is the output of a rendered image and every treestore has an ID) treestoreIDs
/ treestoresIDs
/ treestore IDs
/ treestores IDs
/ treestore's IDs
/ treeStores' IDs
or others... Any suggestion?
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.
I think the way it is now is OK: treestoreID(s)
when functions for getting the IDs fail and treestore(s)
when we refer to the directory (e.g. removing a treestore).
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.
@iaguis ok! Thanks!
A nit but LGTM |
This patch adds an initial image gc command. Image gc can be thought in multiple ways: * Remove treeStores not referenced by any non GCed pod * Remove images not used for some time This patchs implements the removal of treeStores not referenced by any non GCed pod. A "preparelock" is added to avoid races between getting the list of referenced treeStoreIDs and new pods/treeStores being created/referenced in the meantime.
44f564b
to
071d14e
Compare
Thanks @sgotti! |
rkt: implement initial image gc for the tree store.
This is on top of #1240. The last patch is the interesting one.
rkt: implement initial image gc for the tree store.
This patch adds an initial image gc command.
Image gc can be thought in multiple ways:
Cleanup the store from other garbage:Entries removed from the aciinfo tables but failed to remove for any reason from the diskv stores duringrkt image rm
Cleanup store's treestorelocks directoryCleanup store's tmpdir directoryThis patch implements the removal of treeStores not referenced by any non GCed pod.
A "preparelock" is added to avoid races between getting the list of referenced treeStoreIDs and new pods/treeStores being created/referenced in the meantime.