Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't remove sector data when moving data into a shared path #7494

Merged
merged 1 commit into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions extern/sector-storage/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,13 +588,13 @@ func (m *Manager) Remove(ctx context.Context, sector storage.SectorRef) error {

var err error

if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTSealed, true); rerr != nil {
if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTSealed, true, nil); rerr != nil {
err = multierror.Append(err, xerrors.Errorf("removing sector (sealed): %w", rerr))
}
if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTCache, true); rerr != nil {
if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTCache, true, nil); rerr != nil {
err = multierror.Append(err, xerrors.Errorf("removing sector (cache): %w", rerr))
}
if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTUnsealed, true); rerr != nil {
if rerr := m.storage.Remove(ctx, sector.ID, storiface.FTUnsealed, true, nil); rerr != nil {
err = multierror.Append(err, xerrors.Errorf("removing sector (unsealed): %w", rerr))
}

Expand Down
2 changes: 1 addition & 1 deletion extern/sector-storage/piece_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func (p *pieceProviderTestHarness) addRemoteWorker(t *testing.T, tasks []sealtas
func (p *pieceProviderTestHarness) removeAllUnsealedSectorFiles(t *testing.T) {
for i := range p.localStores {
ls := p.localStores[i]
require.NoError(t, ls.Remove(p.ctx, p.sector.ID, storiface.FTUnsealed, false))
require.NoError(t, ls.Remove(p.ctx, p.sector.ID, storiface.FTUnsealed, false, nil))
}
}

Expand Down
8 changes: 4 additions & 4 deletions extern/sector-storage/stores/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (

var log = logging.Logger("stores")

var _ partialFileHandler = &DefaultPartialFileHandler{}
var _ PartialFileHandler = &DefaultPartialFileHandler{}

// DefaultPartialFileHandler is the default implementation of the partialFileHandler interface.
// DefaultPartialFileHandler is the default implementation of the PartialFileHandler interface.
// This is probably the only implementation we'll ever use because the purpose of the
// interface to is to mock out partial file related functionality during testing.
type DefaultPartialFileHandler struct{}
Expand All @@ -46,7 +46,7 @@ func (d *DefaultPartialFileHandler) Close(pf *partialfile.PartialFile) error {

type FetchHandler struct {
Local Store
PfHandler partialFileHandler
PfHandler PartialFileHandler
}

func (handler *FetchHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // /remote/
Expand Down Expand Up @@ -179,7 +179,7 @@ func (handler *FetchHandler) remoteDeleteSector(w http.ResponseWriter, r *http.R
return
}

if err := handler.Local.Remove(r.Context(), id, ft, false); err != nil {
if err := handler.Local.Remove(r.Context(), id, ft, false, []ID{ID(r.FormValue("keep"))}); err != nil {
log.Errorf("%+v", err)
w.WriteHeader(500)
return
Expand Down
14 changes: 7 additions & 7 deletions extern/sector-storage/stores/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestRemoteGetAllocated(t *testing.T) {
tcs := map[string]struct {
piFnc func(pi *pieceInfo)
storeFnc func(s *mocks.MockStore)
pfFunc func(s *mocks.MockpartialFileHandler)
pfFunc func(s *mocks.MockPartialFileHandler)

// expectation
expectedStatusCode int
Expand Down Expand Up @@ -129,7 +129,7 @@ func TestRemoteGetAllocated(t *testing.T) {
storiface.SectorPaths{}, nil).Times(1)
},

pfFunc: func(pf *mocks.MockpartialFileHandler) {
pfFunc: func(pf *mocks.MockPartialFileHandler) {
pf.EXPECT().OpenPartialFile(abi.PaddedPieceSize(sectorSize), pfPath).Return(&partialfile.PartialFile{},
xerrors.New("some error")).Times(1)
},
Expand All @@ -146,7 +146,7 @@ func TestRemoteGetAllocated(t *testing.T) {
storiface.SectorPaths{}, nil).Times(1)
},

pfFunc: func(pf *mocks.MockpartialFileHandler) {
pfFunc: func(pf *mocks.MockPartialFileHandler) {
pf.EXPECT().OpenPartialFile(abi.PaddedPieceSize(sectorSize), pfPath).Return(emptyPartialFile,
nil).Times(1)

Expand All @@ -165,7 +165,7 @@ func TestRemoteGetAllocated(t *testing.T) {
storiface.SectorPaths{}, nil).Times(1)
},

pfFunc: func(pf *mocks.MockpartialFileHandler) {
pfFunc: func(pf *mocks.MockPartialFileHandler) {
pf.EXPECT().OpenPartialFile(abi.PaddedPieceSize(sectorSize), pfPath).Return(emptyPartialFile,
nil).Times(1)

Expand All @@ -184,7 +184,7 @@ func TestRemoteGetAllocated(t *testing.T) {
storiface.SectorPaths{}, nil).Times(1)
},

pfFunc: func(pf *mocks.MockpartialFileHandler) {
pfFunc: func(pf *mocks.MockPartialFileHandler) {
pf.EXPECT().OpenPartialFile(abi.PaddedPieceSize(sectorSize), pfPath).Return(emptyPartialFile,
nil).Times(1)

Expand All @@ -203,7 +203,7 @@ func TestRemoteGetAllocated(t *testing.T) {
defer mockCtrl.Finish()

lstore := mocks.NewMockStore(mockCtrl)
pfhandler := mocks.NewMockpartialFileHandler(mockCtrl)
pfhandler := mocks.NewMockPartialFileHandler(mockCtrl)

handler := &stores.FetchHandler{
lstore,
Expand Down Expand Up @@ -371,7 +371,7 @@ func TestRemoteGetSector(t *testing.T) {
// when test is done, assert expectations on all mock objects.
defer mockCtrl.Finish()
lstore := mocks.NewMockStore(mockCtrl)
pfhandler := mocks.NewMockpartialFileHandler(mockCtrl)
pfhandler := mocks.NewMockPartialFileHandler(mockCtrl)

var path string

Expand Down
2 changes: 2 additions & 0 deletions extern/sector-storage/stores/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ type SectorStorageInfo struct {
Primary bool
}

//go:generate go run github.com/golang/mock/mockgen -destination=mocks/index.go -package=mocks . SectorIndex

type SectorIndex interface { // part of storage-miner api
StorageAttach(context.Context, StorageInfo, fsutil.FsStat) error
StorageInfo(context.Context, ID) (StorageInfo, error)
Expand Down
8 changes: 6 additions & 2 deletions extern/sector-storage/stores/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import (
"github.com/filecoin-project/lotus/extern/sector-storage/storiface"
)

//go:generate go run github.com/golang/mock/mockgen -destination=mocks/pf.go -package=mocks . PartialFileHandler

// PartialFileHandler helps mock out the partial file functionality during testing.
type partialFileHandler interface {
type PartialFileHandler interface {
// OpenPartialFile opens and returns a partial file at the given path and also verifies it has the given
// size
OpenPartialFile(maxPieceSize abi.PaddedPieceSize, path string) (*partialfile.PartialFile, error)
Expand All @@ -30,9 +32,11 @@ type partialFileHandler interface {
Close(pf *partialfile.PartialFile) error
}

//go:generate go run github.com/golang/mock/mockgen -destination=mocks/store.go -package=mocks . Store

type Store interface {
AcquireSector(ctx context.Context, s storage.SectorRef, existing storiface.SectorFileType, allocate storiface.SectorFileType, sealing storiface.PathType, op storiface.AcquireMode) (paths storiface.SectorPaths, stores storiface.SectorPaths, err error)
Remove(ctx context.Context, s abi.SectorID, types storiface.SectorFileType, force bool) error
Remove(ctx context.Context, s abi.SectorID, types storiface.SectorFileType, force bool, keepIn []ID) error

// like remove, but doesn't remove the primary sector copy, nor the last
// non-primary copy if there no primary copies
Expand Down
9 changes: 8 additions & 1 deletion extern/sector-storage/stores/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ func (st *Local) Local(ctx context.Context) ([]StoragePath, error) {
return out, nil
}

func (st *Local) Remove(ctx context.Context, sid abi.SectorID, typ storiface.SectorFileType, force bool) error {
func (st *Local) Remove(ctx context.Context, sid abi.SectorID, typ storiface.SectorFileType, force bool, keepIn []ID) error {
if bits.OnesCount(uint(typ)) != 1 {
return xerrors.New("delete expects one file type")
}
Expand All @@ -558,7 +558,14 @@ func (st *Local) Remove(ctx context.Context, sid abi.SectorID, typ storiface.Sec
return xerrors.Errorf("can't delete sector %v(%d), not found", sid, typ)
}

storeLoop:
for _, info := range si {
for _, id := range keepIn {
if id == info.ID {
continue storeLoop
}
}

if err := st.removeSector(ctx, sid, typ, info.ID); err != nil {
return err
}
Expand Down
58 changes: 29 additions & 29 deletions extern/sector-storage/stores/mocks/index.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading