Skip to content

Commit

Permalink
Drop errgroup.WithContext and add withCause (#1380)
Browse files Browse the repository at this point in the history
We are sometimes unable to expand an APK into our cache due to an opaque
"context canceled" error. This change attempts to address that in two
ways.

First, we drop our use of errgroup.WithContext when installing packages.
If one package fails to install, we'll let the rest run to completion so
that we make more forward progress towards populating the cache (if
there is one). Even though the overall operation will still fail, in
cases where that failure was transient, we avoid throwing away in-flight
work towards the other packages.

Second, we'll try to extract a cause for the context cancellation and
annotate the errgroup's error with it so that we know why the overall
operation was canceled.

At least one of these changes should help determine what's going on.

Signed-off-by: Jon Johnson <[email protected]>
  • Loading branch information
jonjohnsonjr authored Oct 31, 2024
1 parent e707968 commit c8b52a0
Showing 1 changed file with 20 additions and 10 deletions.
30 changes: 20 additions & 10 deletions pkg/apk/apk/implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func (a *APK) CalculateWorld(ctx context.Context, allpkgs []*RepositoryPackage)
// TODO: Consider making this configurable option.
jobs := runtime.GOMAXPROCS(0)

g, gctx := errgroup.WithContext(ctx)
var g errgroup.Group
g.SetLimit(jobs + 1)

resolved := make([]*APKResolved, len(allpkgs))
Expand All @@ -535,11 +535,11 @@ func (a *APK) CalculateWorld(ctx context.Context, allpkgs []*RepositoryPackage)
i, pkg := i, pkg

g.Go(func() error {
r, err := a.FetchPackage(gctx, pkg)
r, err := a.FetchPackage(ctx, pkg)
if err != nil {
return fmt.Errorf("fetching %s: %w", pkg.Name, err)
}
res, err := ResolveApk(gctx, r)
res, err := ResolveApk(ctx, r)
if err != nil {
return fmt.Errorf("resolving %s: %w", pkg.Name, err)
}
Expand All @@ -554,12 +554,22 @@ func (a *APK) CalculateWorld(ctx context.Context, allpkgs []*RepositoryPackage)
}

if err := g.Wait(); err != nil {
return nil, fmt.Errorf("installing packages: %w", err)
return nil, fmt.Errorf("calculating world: %w", withCause(ctx, err))
}

return resolved, nil
}

// Sometimes we get an opaque error about context cancellation, and it's unclear what caused it.
// If we get something useful from ctx via context.Cause, we'll annotate err with it.
func withCause(ctx context.Context, err error) error {
if cause := context.Cause(ctx); cause != nil {
return fmt.Errorf("%w: %w", err, cause)
}

return err
}

func (a *APK) ResolveAndCalculateWorld(ctx context.Context) ([]*APKResolved, error) {
log := clog.FromContext(ctx)
log.Debug("resolving and calculating 'world' (packages to install)")
Expand Down Expand Up @@ -625,7 +635,7 @@ func (a *APK) InstallPackages(ctx context.Context, sourceDateEpoch *time.Time, a
// TODO: Consider making this configurable option.
jobs := runtime.GOMAXPROCS(0)

g, gctx := errgroup.WithContext(ctx)
var g errgroup.Group
g.SetLimit(jobs + 1)

expanded := make([]*expandapk.APKExpanded, len(allpkgs))
Expand All @@ -648,8 +658,8 @@ func (a *APK) InstallPackages(ctx context.Context, sourceDateEpoch *time.Time, a
g.Go(func() error {
for i, ch := range done {
select {
case <-gctx.Done():
return gctx.Err()
case <-ctx.Done():
return ctx.Err()
case <-ch:
exp := expanded[i]
pkg := allpkgs[i]
Expand All @@ -670,7 +680,7 @@ func (a *APK) InstallPackages(ctx context.Context, sourceDateEpoch *time.Time, a
}
infos[i] = pkgInfo

installedFiles, err := a.installPackage(gctx, pkgInfo, exp, sourceDateEpoch)
installedFiles, err := a.installPackage(ctx, pkgInfo, exp, sourceDateEpoch)
if err != nil {
return fmt.Errorf("installing %s: %w", pkg, err)
}
Expand All @@ -688,7 +698,7 @@ func (a *APK) InstallPackages(ctx context.Context, sourceDateEpoch *time.Time, a
i, pkg := i, pkg

g.Go(func() error {
exp, err := a.expandPackage(gctx, pkg)
exp, err := a.expandPackage(ctx, pkg)
if err != nil {
return fmt.Errorf("expanding %s: %w", pkg, err)
}
Expand All @@ -701,7 +711,7 @@ func (a *APK) InstallPackages(ctx context.Context, sourceDateEpoch *time.Time, a
}

if err := g.Wait(); err != nil {
return fmt.Errorf("installing packages: %w", err)
return fmt.Errorf("installing packages: %w", withCause(ctx, err))
}

// update the installed file
Expand Down

0 comments on commit c8b52a0

Please sign in to comment.