From d39d9ed60b78667e60922868c4763f16c9d66555 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Mon, 20 Mar 2017 14:46:02 -0400 Subject: [PATCH] gc: address CR comments License: MIT Signed-off-by: Kevin Atkinson --- core/commands/repo.go | 1 + core/corerepo/gc.go | 5 +++++ pin/gc/gc.go | 30 +++++++++++++++++------------- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/core/commands/repo.go b/core/commands/repo.go index b34010e66..4a6fc2844 100644 --- a/core/commands/repo.go +++ b/core/commands/repo.go @@ -40,6 +40,7 @@ var RepoCmd = &cmds.Command{ }, } +// GcResult is the result returned by "repo gc" command. type GcResult struct { Key *cid.Cid Error string `json:",omitempty"` diff --git a/core/corerepo/gc.go b/core/corerepo/gc.go index 4bf45f829..b02ee2bb4 100644 --- a/core/corerepo/gc.go +++ b/core/corerepo/gc.go @@ -91,6 +91,9 @@ func GarbageCollect(n *core.IpfsNode, ctx context.Context) error { return CollectResult(ctx, rmed, nil) } +// CollectResult collects the output of a garbage collection run and calls the +// given callback for each object removed. It also collects all errors into a +// MultiError which is returned after the gc is completed. func CollectResult(ctx context.Context, gcOut <-chan gc.Result, cb func(*cid.Cid)) error { var errors []error loop: @@ -121,10 +124,12 @@ loop: } } +// NewMultiError creates a new MultiError object from a given slice of errors. func NewMultiError(errs ...error) *MultiError { return &MultiError{errs[:len(errs)-1], errs[len(errs)-1]} } +// MultiError contains the results of multiple errors. type MultiError struct { Errors []error Summary error diff --git a/pin/gc/gc.go b/pin/gc/gc.go index ccf354b45..4a990da9a 100644 --- a/pin/gc/gc.go +++ b/pin/gc/gc.go @@ -16,6 +16,8 @@ import ( var log = logging.Logger("gc") +// Result represents an incremental output from a garbage collection +// run. It contains either an error, or the cid of a removed object. type Result struct { KeyRemoved *cid.Cid Error error @@ -66,7 +68,7 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, ls dag.LinkService, pn pin. err := bs.DeleteBlock(k) if err != nil { errors = true - output <- Result{Error: &CouldNotDeleteBlockError{k, err}} + output <- Result{Error: &CannotDeleteBlockError{k, err}} //log.Errorf("Error removing key from blockstore: %s", err) // continue as error is non-fatal continue loop @@ -82,7 +84,7 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, ls dag.LinkService, pn pin. } } if errors { - output <- Result{Error: ErrCouldNotDeleteSomeBlocks} + output <- Result{Error: ErrCannotDeleteSomeBlocks} } }() @@ -103,6 +105,8 @@ func Descendants(ctx context.Context, getLinks dag.GetLinks, set *cid.Set, roots return nil } +// ColoredSet computes the set of nodes in the graph that are pinned by the +// pins in the given pinner. func ColoredSet(ctx context.Context, pn pin.Pinner, ls dag.LinkService, bestEffortRoots []*cid.Cid, output chan<- Result) (*cid.Set, error) { // KeySet currently implemented in memory, in the future, may be bloom filter or // disk backed to conserve memory. @@ -112,7 +116,7 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ls dag.LinkService, bestEffo links, err := ls.GetLinks(ctx, cid) if err != nil { errors = true - output <- Result{Error: &CouldNotFetchLinksError{cid, err}} + output <- Result{Error: &CannotFetchLinksError{cid, err}} } return links, nil } @@ -126,7 +130,7 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ls dag.LinkService, bestEffo links, err := ls.GetLinks(ctx, cid) if err != nil && err != dag.ErrNotFound { errors = true - output <- Result{Error: &CouldNotFetchLinksError{cid, err}} + output <- Result{Error: &CannotFetchLinksError{cid, err}} } return links, nil } @@ -147,30 +151,30 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ls dag.LinkService, bestEffo } if errors { - return nil, ErrCouldNotFetchAllLinks - } else { - return gcs, nil + return nil, ErrCannotFetchAllLinks } + + return gcs, nil } -var ErrCouldNotFetchAllLinks = errors.New("garbage collection aborted: could not retrieve some links") +var ErrCannotFetchAllLinks = errors.New("garbage collection aborted: could not retrieve some links") -var ErrCouldNotDeleteSomeBlocks = errors.New("garbage collection incomplete: could not delete some blocks") +var ErrCannotDeleteSomeBlocks = errors.New("garbage collection incomplete: could not delete some blocks") -type CouldNotFetchLinksError struct { +type CannotFetchLinksError struct { Key *cid.Cid Err error } -func (e *CouldNotFetchLinksError) Error() string { +func (e *CannotFetchLinksError) Error() string { return fmt.Sprintf("could not retrieve links for %s: %s", e.Key, e.Err) } -type CouldNotDeleteBlockError struct { +type CannotDeleteBlockError struct { Key *cid.Cid Err error } -func (e *CouldNotDeleteBlockError) Error() string { +func (e *CannotDeleteBlockError) Error() string { return fmt.Sprintf("could not remove %s: %s", e.Key, e.Err) }