release-5.32 Zstd backports #2507
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We are already calling m.LayerInfos() anyway, so there is ~no extra cost. And using LayerInfos means we don't need to worry about reversing the order of layers, and we will have access to the layer index, allowing us to acccess the indexTo* fields in the future. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
- Don't claim that we only use compressed digests. - Explicitly document that we assume TOC digests to be unambiguous - Actually define the term "DiffID". - Be more precise in computeID about the criteria being layer identity, not where we pull the layer from. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Some errors are severe enough that just logging and continuing is not really worthwhile. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…tyDataLocked Currrently we "only" have indexToTOCDigest and blobDiffIDs, but we will make this more complex. Centralizing the consumption of these fields into trustedLayerIdentityDataLocked ensure that all consumers interpret the data exactly consistently (and it also allows us to use a single "trusted" variable instead of 2/3 individual ones). Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The new code is not called, so it should not change behavior (apart from extending the BoltDB/SQLite schema). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…storage by DiffID If we can, prefer identifying layers by DiffID, because multiple TOCs can map to the same DiffID; and because it maximizes reuse with non-TOC layers. For now, the new situation is unreachable. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We will add one more instance of this, so share the code. Should not change behavior (it does remove one unreachable code path). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
… is known
- Multiple TOC values might correspond to a single DiffID (e.g. if different
compression levels are used); try to share them all, identified by DiffID
(so that we also reuse with non-TOC pulls).
- LayersByTOCDigest only uses a single TOC digest per layer; BlobInfoCache
allows multiple matches, matches layers which have been since deleted,
and potentially matches TOC digests which we have created by pushing
but haven't pulled yet.
- On reuse, we can now use DiffID-based layer identities even if the reuse
was TOC~driven.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…yers Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…ayers - Rely on it instead of triggering the "untrusted DiffID" logic - Also propagate it to storage Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…er match The rules expect us to set manifest editing updates. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... and add CandidateWithLocation and CandidateWithUnknownLocation , so that the BIC implementations only need to deal with one value instead of carrying around three; we will want to add one more, and encapsulating them all into a single template will make it transparent to the cache implementations. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... just because we now can, and to nudge all future caches to be designed around CandidateTemplateWithCompression. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We will add more logic to the default case, so sharing the CandidateCompressionMatchesReuseConditions call is not going to be as easy. Split the two code paths. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We will want to record more than a single alghoritm name. For now, just introduce the structure and modify users, we'll add the new fields later. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
… blobs ... because we don't trust the TOC data, if any. This allows us to remove the zstd:chunked hack; we, at least, now record those blobs as zstd. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
If we don't know an uncompressed digest, don't try using "". Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The cache implementations are recording both the base and specific compression variant; CandidateLocations2 all call CandidateTemplateWithCompression to choose the appropriate variants to return based on CandidateLocations2Options. This way, neither the BIC implementations nor the transports are not responsible for converting zstd:chunked entries to zstd entries if the user wants the latter. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Introduce distinct uploadedCompressorBaseVariantName and uploadedCompressorSpecificVariantName fields; that way we now never call RecordDigestCompressorData with inconsistent zstd / zstd:chunked in one field, so we can always record data when we see, or create, a zstd:chunked layer, removing the current hack. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
- Add a CompressionAnnotations field - Allow turning a known-zstd blob into a zstd:chunked one if we know the right annotations This just adds the fields, nothing sets them yet, should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
- Return the required annotations, if we have them - If we have a zstd blob and the BIC contains the annotations, we don't check for the blob's presence initially. In that case, don't skip it if we find the BIC annotations. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... instead of only treating it as zstd. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
Wrong target, I’ll re-create the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is #2321 + #2503 + #2487 , backported to a newly-created
release-5.32branch, along with a release bump to 5.32.1.Alternatively, we could just tag
mainas a5.32.1without backporting: see https://github.com/containers/image/compare/main..mtrmac:5.32-zstd , the difference is a set of typo fixes and dependency updates. The dependency updates are probably unwanted, I’m not sure.Cc: @TomSweeneyRedHat