Skip to content

Commit 41c868b

Browse files
authored
Merge pull request moby#4947 from tonistiigi/temp-lease-perf
llbsolver: create single temp lease for exports for performance
2 parents 593aad1 + 7b52fed commit 41c868b

File tree

3 files changed

+32
-7
lines changed

3 files changed

+32
-7
lines changed

cache/remote.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (sr *immutableRef) GetRemotes(ctx context.Context, createIfNeeded bool, ref
5252
}
5353

5454
// Search all available remotes that has the topmost blob with the specified
55-
// compression with all combination of copmressions
55+
// compression with all combination of compressions
5656
res := []*solver.Remote{remote}
5757
topmost, parentChain := remote.Descriptors[len(remote.Descriptors)-1], remote.Descriptors[:len(remote.Descriptors)-1]
5858
vDesc, err := getBlobWithCompression(ctx, sr.cm.ContentStore, topmost, refCfg.Compression.Type)

solver/llbsolver/history.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,11 +366,11 @@ func (h *HistoryQueue) addResource(ctx context.Context, l leases.Lease, desc *co
366366
}
367367
if _, err := h.hContentStore.Info(ctx, desc.Digest); err != nil {
368368
if errdefs.IsNotFound(err) {
369-
ctx, release, err := leaseutil.WithLease(ctx, h.hLeaseManager, leases.WithID("history_migration_"+identity.NewID()), leaseutil.MakeTemporary)
369+
lr, ctx, err := leaseutil.NewLease(ctx, h.hLeaseManager, leases.WithID("history_migration_"+identity.NewID()), leaseutil.MakeTemporary)
370370
if err != nil {
371371
return err
372372
}
373-
defer release(ctx)
373+
defer lr.Discard()
374374
ok, err := h.migrateBlobV2(ctx, string(desc.Digest), detectSkipLayers)
375375
if err != nil {
376376
return err

solver/llbsolver/solver.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/moby/buildkit/util/compression"
3535
"github.com/moby/buildkit/util/entitlements"
3636
"github.com/moby/buildkit/util/grpcerrors"
37+
"github.com/moby/buildkit/util/leaseutil"
3738
"github.com/moby/buildkit/util/progress"
3839
"github.com/moby/buildkit/util/tracing/detect"
3940
"github.com/moby/buildkit/worker"
@@ -156,7 +157,7 @@ func (s *Solver) Bridge(b solver.Builder) frontend.FrontendLLBBridge {
156157
return s.bridge(b)
157158
}
158159

159-
func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend.SolveRequest, exp ExporterRequest, j *solver.Job, usage *resources.SysSampler) (func(*Result, []exporter.DescriptorReference, error) error, error) {
160+
func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend.SolveRequest, exp ExporterRequest, j *solver.Job, usage *resources.SysSampler) (func(context.Context, *Result, []exporter.DescriptorReference, error) error, error) {
160161
stopTrace, err := detect.Recorder.Record(ctx)
161162
if err != nil {
162163
return nil, err
@@ -184,7 +185,7 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend
184185
return nil, err
185186
}
186187

187-
return func(res *Result, descrefs []exporter.DescriptorReference, err error) error {
188+
return func(ctx context.Context, res *Result, descrefs []exporter.DescriptorReference, err error) error {
188189
en := time.Now()
189190
rec.CompletedAt = &en
190191

@@ -197,7 +198,7 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend
197198
}
198199
}
199200

200-
ctx, cancel := context.WithCancelCause(context.Background())
201+
ctx, cancel := context.WithCancelCause(ctx)
201202
ctx, _ = context.WithTimeoutCause(ctx, 300*time.Second, errors.WithStack(context.DeadlineExceeded))
202203
defer cancel(errors.WithStack(context.Canceled))
203204

@@ -509,7 +510,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
509510
return nil, err1
510511
}
511512
defer func() {
512-
err = rec(resProv, descrefs, err)
513+
err = rec(context.WithoutCancel(ctx), resProv, descrefs, err)
513514
}()
514515
}
515516

@@ -585,6 +586,22 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
585586
return nil, err
586587
}
587588

589+
// Functions that create new objects in containerd (eg. content blobs) need to have a lease to ensure
590+
// that the object is not garbage collected immediately. This is protected by the indivual components,
591+
// but because creating a lease is not cheap and requires a disk write, we create a single lease here
592+
// early and let all the exporters, cache export and provenance creation use the same one.
593+
lm, err := s.leaseManager()
594+
if err != nil {
595+
return nil, err
596+
}
597+
ctx, done, err := leaseutil.WithLease(ctx, lm, leaseutil.MakeTemporary)
598+
if err != nil {
599+
return nil, err
600+
}
601+
releasers = append(releasers, func() {
602+
done(context.WithoutCancel(ctx))
603+
})
604+
588605
cacheExporters, inlineCacheExporter := splitCacheExporters(exp.CacheExporters)
589606

590607
var exporterResponse map[string]string
@@ -747,6 +764,14 @@ func (s *Solver) runExporters(ctx context.Context, exporters []exporter.Exporter
747764
return exporterResponse, descs, nil
748765
}
749766

767+
func (s *Solver) leaseManager() (*leaseutil.Manager, error) {
768+
w, err := defaultResolver(s.workerController)()
769+
if err != nil {
770+
return nil, err
771+
}
772+
return w.LeaseManager(), nil
773+
}
774+
750775
func splitCacheExporters(exporters []RemoteCacheExporter) (rest []RemoteCacheExporter, inline inlineCacheExporter) {
751776
rest = make([]RemoteCacheExporter, 0, len(exporters))
752777
for _, exp := range exporters {

0 commit comments

Comments
 (0)