store: decouple aci store and treestore implementations. #2919
Conversation
a72b77b to
9449d4b
Compare
pkg/backup/backup.go
Outdated
| ) | ||
|
|
||
| // createBackup backs a database up in a given directory. It basically | ||
| // createBackup backs a directory up in a given directory. It basically |
There was a problem hiding this comment.
// CreateBackup (uppercase)
|
Just nits really, I am not able to overview the technical specialties in all detail, but from a birds-eye this makes sense 👍 |
|
@s-urbaniak Thanks for the review, I'll address all your suggestions. If you have any doubts around the current architecture, proposals or anything else I'd be happy to talk about them. In #2911 I tried to document this and the primary points that brought to the current state. |
80d8e08 to
c4d8d67
Compare
For a strange reason store.NewStore was made to get the rkt DataDir and calculate by itself the store dir adding the "cas" directory. This patch makes it more logical passing to NewStore the store dir.
c4d8d67 to
f11ba81
Compare
f11ba81 to
e810f60
Compare
|
Can you PTAL at this since other fixes depend on this (and it's a pain to rebase it on every change to the store like the last ones 😄)? If there's something not clear on the decoupling reasons please let me know (see also #2911). Thanks! |
e810f60 to
e113492
Compare
|
|
||
| tsSizes[key] = tsSize | ||
| } | ||
| } |
There was a problem hiding this comment.
I removed this since with the decoupling the store cannot call treestore functions (and it's used only on migration from quite old rkt versions). #2968 proposed fix will cover this.
|
LGTM, once green 👍 nits: missing godoc's in a couple of places (can be done in a subsequent PR), and still many getters, but this is out of scope here ;-) |
|
In the message of the last commit: s/This patch split the store/This patch splits the store/ Please wrap TODOs |
The current image store and tree store are logically decoupled but
historically the implementation was done under an unique store object.
This patch splits the store in its two logical parts: an image store
(currently only appc ACIs) and a tree store.
This makes the code cleaner and will ease implementations of different kind
of image types for other specs (like OCI).
Future patches will cover the various TODOs:
* Migration to a less unfortunate path naming ("cas") and separated
paths for the store and the tree store. This needs to be seamless and
is quite complex (have to handle pods with an overlayfs mounted etc.)
Make the tree store accept an interface to an image store (this needs
* The removal of the current logic of saving the aci tree store sizes
inside the aci store totally decoupling them and make `rkt image list`
calculate the total size)
e113492 to
3709ddc
Compare
|
LGTM if green |
|
The failure on rawhide is more like a jenkins failure |
The last patch is the primary one.
The current aci store and treestore are logically decoupled but historically
the implementation was done under an unique store object.
This patch splits the store in its two logical parts: an aci store and an aci
tree store.
This makes the code cleaner and will ease implementations of different kind
of aci stores (previous definition of a common store interface) and also
additional store types for other specs (like OCI).
Future patches will cover the various TODOs:
rkt image listcalculate the total size)