storage
storage copied to clipboard
RFC: Move the tar-split digest into the TOC
A minimal prototype for a fix of #1888 .
Note that this would immediately break pulling all currently-existing layers.
Also this probably increases memory requirements because the TOC exists as individual Go values longer.
Cc: @giuseppe
@giuseppe @mtrmac What should we do with this one?
#1888 must have a solution. This is one possible solution; it also breaks all currently-existing zstd:chunked images, which might be fine or very bad, I don’t know which one it is.
#1888 must have a solution. This is one possible solution; it also breaks all currently-existing zstd:chunked images, which might be fine or very bad, I don’t know which one it is.
It is fine to break these images now.
The tarsplit data must not be mandatory. If the tarsplit data is missing, we should just skip fetching it (and deal with errors to recreate the original image later)
With this PR:
- If there is no tar-split offset, the image is accepted
- If there is a tar-split offset and the new tar-split digest in the TOC, the image is accepted
- If there is a tar-split offset and the TOC does not have the new digest, the image is rejected (most recently-created images are like this)
Alternatively, in the last case, we could accept the layer and ignore the tar-split data. If we do that, the images will continue to pull fine; but previously they also pushed fine and that would now break.
Marking ready for review now.
- If there is a tar-split offset and the TOC does not have the new digest, the image is rejected (most recently-created images are like this)
could we just drop it from the image generation and always ignore it? So we don't have to worry about it in future
could we just drop it from the image generation and always ignore it? So we don't have to worry about it in future
I’m not sure which part you mean should be dropped. We need:
- the offset to be provided somehow. Currently it is an annotation. (It could be in the TOC instead.)
- a digest, in the TOC. (That’s being added in this PR. The previous digest, in the annotation, is still being added, but it is now ignored).
We can’t drop the tar-split offset. Do you mean to stop adding the TarSplitChecksumKey annotation? Something else?
We can’t drop the tar-split offset. Do you mean to stop adding the
TarSplitChecksumKeyannotation? Something else?
yes, to stop doing that and use only the information in the TOC
Done.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: giuseppe, mtrmac
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [giuseppe,mtrmac]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/lgtm