From 7b52fed89f9acbf4f908f151647609363dba119d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 23 May 2024 09:44:15 -0700 Subject: [PATCH] llbsolver: create single temp lease for exports for performance Signed-off-by: Tonis Tiigi --- cache/remote.go | 2 +- solver/llbsolver/history.go | 4 ++-- solver/llbsolver/solver.go | 33 +++++++++++++++++++++++++++++---- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/cache/remote.go b/cache/remote.go index fa2aab2a57e7..4f5fd091f6f5 100644 --- a/cache/remote.go +++ b/cache/remote.go @@ -52,7 +52,7 @@ func (sr *immutableRef) GetRemotes(ctx context.Context, createIfNeeded bool, ref } // Search all available remotes that has the topmost blob with the specified - // compression with all combination of copmressions + // compression with all combination of compressions res := []*solver.Remote{remote} topmost, parentChain := remote.Descriptors[len(remote.Descriptors)-1], remote.Descriptors[:len(remote.Descriptors)-1] vDesc, err := getBlobWithCompression(ctx, sr.cm.ContentStore, topmost, refCfg.Compression.Type) diff --git a/solver/llbsolver/history.go b/solver/llbsolver/history.go index 13aed089d331..e62549ca371c 100644 --- a/solver/llbsolver/history.go +++ b/solver/llbsolver/history.go @@ -366,11 +366,11 @@ func (h *HistoryQueue) addResource(ctx context.Context, l leases.Lease, desc *co } if _, err := h.hContentStore.Info(ctx, desc.Digest); err != nil { if errdefs.IsNotFound(err) { - ctx, release, err := leaseutil.WithLease(ctx, h.hLeaseManager, leases.WithID("history_migration_"+identity.NewID()), leaseutil.MakeTemporary) + lr, ctx, err := leaseutil.NewLease(ctx, h.hLeaseManager, leases.WithID("history_migration_"+identity.NewID()), leaseutil.MakeTemporary) if err != nil { return err } - defer release(ctx) + defer lr.Discard() ok, err := h.migrateBlobV2(ctx, string(desc.Digest), detectSkipLayers) if err != nil { return err diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index 5d20cea38701..7e84d8383585 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -34,6 +34,7 @@ import ( "github.com/moby/buildkit/util/compression" "github.com/moby/buildkit/util/entitlements" "github.com/moby/buildkit/util/grpcerrors" + "github.com/moby/buildkit/util/leaseutil" "github.com/moby/buildkit/util/progress" "github.com/moby/buildkit/util/tracing/detect" "github.com/moby/buildkit/worker" @@ -156,7 +157,7 @@ func (s *Solver) Bridge(b solver.Builder) frontend.FrontendLLBBridge { return s.bridge(b) } -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) { +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) { stopTrace, err := detect.Recorder.Record(ctx) if err != nil { return nil, err @@ -184,7 +185,7 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend return nil, err } - return func(res *Result, descrefs []exporter.DescriptorReference, err error) error { + return func(ctx context.Context, res *Result, descrefs []exporter.DescriptorReference, err error) error { en := time.Now() rec.CompletedAt = &en @@ -197,7 +198,7 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend } } - ctx, cancel := context.WithCancelCause(context.Background()) + ctx, cancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 300*time.Second, errors.WithStack(context.DeadlineExceeded)) defer cancel(errors.WithStack(context.Canceled)) @@ -509,7 +510,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro return nil, err1 } defer func() { - err = rec(resProv, descrefs, err) + err = rec(context.WithoutCancel(ctx), resProv, descrefs, err) }() } @@ -585,6 +586,22 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro return nil, err } + // Functions that create new objects in containerd (eg. content blobs) need to have a lease to ensure + // that the object is not garbage collected immediately. This is protected by the indivual components, + // but because creating a lease is not cheap and requires a disk write, we create a single lease here + // early and let all the exporters, cache export and provenance creation use the same one. + lm, err := s.leaseManager() + if err != nil { + return nil, err + } + ctx, done, err := leaseutil.WithLease(ctx, lm, leaseutil.MakeTemporary) + if err != nil { + return nil, err + } + releasers = append(releasers, func() { + done(context.WithoutCancel(ctx)) + }) + cacheExporters, inlineCacheExporter := splitCacheExporters(exp.CacheExporters) var exporterResponse map[string]string @@ -747,6 +764,14 @@ func (s *Solver) runExporters(ctx context.Context, exporters []exporter.Exporter return exporterResponse, descs, nil } +func (s *Solver) leaseManager() (*leaseutil.Manager, error) { + w, err := defaultResolver(s.workerController)() + if err != nil { + return nil, err + } + return w.LeaseManager(), nil +} + func splitCacheExporters(exporters []RemoteCacheExporter) (rest []RemoteCacheExporter, inline inlineCacheExporter) { rest = make([]RemoteCacheExporter, 0, len(exporters)) for _, exp := range exporters {