solver: implement content based cache support #91
Conversation
tonistiigi
force-pushed
the
contenthash2
branch
from
2a12e90 to
cd1f4ed
Compare
| @@ -92,7 +92,7 @@ func TestChecksumBasicFile(t *testing.T) { | |||
| dgst, err = cc.Checksum(context.TODO(), ref, "/") | |||
| assert.NoError(t, err) | |||
|
|
|||
| assert.Equal(t, digest.Digest("sha256:0d87c8c2a606f961483cd4c5dc0350a4136a299b4066eea4a969d6ed756614cd"), dgst) | |||
| assert.Equal(t, digest.Digest("sha256:7378af5287e8b417b6cbc63154d300e130983bfc645e35e86fdadf6f5060468a"), dgst) | |||
There was a problem hiding this comment.
Because of a seeker bug, record for foo was missing from this hash.
There was a problem hiding this comment.
Do you plan to fix the seeker bug in this PR or a separate PR?
There was a problem hiding this comment.
The bug was in contenthash pkg and is in fixed in this PR. The digest changed because the previous digest was wrong. I misunderstood what the SeekPrefix function did in iradix package and the output of the function could not be used for walking through directory items.
| github.com/hashicorp/go-immutable-radix 826af9ccf0feeee615d546d69b11f8e98da8c8f1 git://github.com/tonistiigi/go-immutable-radix.git |
There was a problem hiding this comment.
Do you plan to propose tonistiigi/go-immutable-radix@826af9c to the upstream?
There was a problem hiding this comment.
Not sure. Another possibility would be to search for some other library. I haven't benchmarked if radix tree is optimal for this case.
|
Please rebase to make sure Windows binaries can be compiled on CI? |
Signed-off-by: Tonis Tiigi <[email protected]>
tonistiigi
force-pushed
the
contenthash2
branch
from
cd1f4ed to
8738929
Compare
|
Rebased, green, merging |
Add ShutdownIfIdle call to buildkit
fixes #86
fixes #58
fixes #35
This needed quite a big refactoring of the solver. Not fully convinced there isn't a better abstraction for this but all others I tried had even worse issues. It is important that once the content based hit is determined, the matched node itself doesn't need to exist in the cache if there is some top level node that has cache instead. This will let us do remote backend for the instruction cache and pull in as little data as possible on cache import.
@AkihiroSuda
Signed-off-by: Tonis Tiigi [email protected]