-
Notifications
You must be signed in to change notification settings - Fork 66
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
a6ffee9
commit d02950b
Showing
3 changed files
with
158 additions
and
1 deletion.
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,154 @@ | ||
- commits: | ||
- subject: "Bugfix: concatReadSeekCloser.Read() with buffers of any size" | ||
hash: 84471473ce02a0b8820e13105f845b67c19903c9 | ||
body: > | ||
Previously, `concatReadSeekCloser.Read()` would incorrectly return | ||
an `io.ErrUnexpectedEOF` if the last read from the second concatenated | ||
`Reader` didn't completely fill the passed buffer. | ||
For instance: | ||
``` | ||
First Reader Second Reader | ||
|aaaaaaaaaaaaa|bbbbbbbbbbbbbbbbbbb| <--- concatReadSeekCloser | ||
|--- previously read ---|--- buffer ---| <--- last read | ||
|xxxx| <--- "excess" buffer | ||
``` | ||
In this example, we have a `concatReadSeekCloser` that concatenates two | ||
`Reader`s (`aaa...` and `bbb...`). The last `Read()` used a buffer | ||
larger than the yet-to-be-read portion of the `bbb...`. So, it would | ||
incorrectly return an `io.ErrUnexpectedEOF`. | ||
This commit makes sure that last `Read()` returns all the remaining data | ||
without an error. It also adds various test cases for | ||
`concatReadSeekCloser.Read()`, many of which would fail before this | ||
correction. | ||
Interestingly, this bug was silently affecting us. Not in a fatal way, | ||
but causing deltas to be larger than necessary. Indeed, running | ||
`TestDeltaSize` after this commit shows that some test cases are | ||
producing deltas smaller than what we expected before. For posterity, | ||
see all the details below. | ||
We use `concatReadSeekCloser`s to concatenate all layers of the basis | ||
image when creating the "signature" of the basis image. In this process, | ||
the `concatReadSeekCloser`s are wrapped around by a buffered reader with | ||
a buffer of 65kB. | ||
If, in any read, part of this 65kB buffer was beyond the second | ||
concatenated reader, it would result in an `io.ErrUnexpectedEOF`. This | ||
would not cause the whole process to fail, but would prematurely end the | ||
signature generation: some of the final blocks in the basis image would | ||
not be added to the signature. Therefore, if those blocks appeared in | ||
the target image, they'd result in (larger) LITERAL, instead of | ||
(smaller) COPY operations. | ||
For illustration, here's the delta generated for the `delta-006-008` | ||
test case. First before this commit: | ||
``` | ||
OP_COPY_N1_N2 0 512 | ||
OP_COPY_N2_N2 1536 1024 | ||
OP_COPY_N2_N2 3584 1024 | ||
OP_COPY_N2_N2 5632 1536 | ||
OP_COPY_N2_N2 8192 1536 | ||
OP_COPY_N2_N2 10752 4096 | ||
OP_COPY_N2_N2 15872 8704 | ||
OP_COPY_N2_N2 25600 10752 | ||
OP_COPY_N2_N2 37376 10752 | ||
OP_COPY_N2_N4 49152 131584 | ||
OP_COPY_N4_N4 181760 150528 | ||
OP_COPY_N4_N4 333312 500736 | ||
OP_COPY_N4_N4 835072 1000960 | ||
OP_COPY_N4_N4 1837056 1000960 | ||
OP_COPY_N4_N4 2839040 1027584 # Here: a COPY op... | ||
OP_LITERAL_N2 21504 # ...followed by a 21 kB LITERAL. | ||
OP_COPY_N4_N2 2838528 512 | ||
OP_COPY_N4_N2 2838528 512 | ||
OP_END | ||
``` | ||
And after this commit: | ||
``` | ||
OP_COPY_N1_N2 0 512 | ||
OP_COPY_N2_N2 1536 1024 | ||
OP_COPY_N2_N2 3584 1024 | ||
OP_COPY_N2_N2 5632 1536 | ||
OP_COPY_N2_N2 8192 1536 | ||
OP_COPY_N2_N2 10752 4096 | ||
OP_COPY_N2_N2 15872 8704 | ||
OP_COPY_N2_N2 25600 10752 | ||
OP_COPY_N2_N2 37376 10752 | ||
OP_COPY_N2_N4 49152 131584 | ||
OP_COPY_N4_N4 181760 150528 | ||
OP_COPY_N4_N4 333312 500736 | ||
OP_COPY_N4_N4 835072 1000960 | ||
OP_COPY_N4_N4 1837056 1000960 | ||
OP_COPY_N4_N4 2839040 1049088 # COPY only, since we detected a longer match | ||
OP_COPY_N4_N2 3888640 512 | ||
OP_COPY_N4_N2 3888640 512 | ||
OP_END | ||
``` | ||
That 21kB LITERAL is the difference in size we saw in the test results. | ||
footer: | ||
Signed-off-by: Leandro Motta Barros <[email protected]> | ||
signed-off-by: Leandro Motta Barros <[email protected]> | ||
Change-type: patch | ||
change-type: patch | ||
author: Leandro Motta Barros | ||
nested: [] | ||
- subject: Minor code and comments tweaks | ||
hash: 7dd51428c918980163fbe34511152dc72e7a3372 | ||
body: | | ||
Using `defer` for the sake of being more idiomatic (and maybe slightly | ||
more reliable); plus, using the proper doc comment standards. | ||
footer: | ||
Signed-off-by: Leandro Motta Barros <[email protected]> | ||
signed-off-by: Leandro Motta Barros <[email protected]> | ||
Change-type: patch | ||
change-type: patch | ||
author: Leandro Motta Barros | ||
nested: [] | ||
version: 20.10.37 | ||
title: "" | ||
date: 2023-06-26T13:44:22.217Z | ||
- commits: | ||
- subject: Further improve resilience of image pulls | ||
hash: d864e340bfe050144252db8b0de9c66a3a40fa20 | ||
|
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
20.10.36 | ||
20.10.37 |