diff --git a/extern/sector-storage/stores/remote.go b/extern/sector-storage/stores/remote.go index 45f4f508b..1e1a54d47 100644 --- a/extern/sector-storage/stores/remote.go +++ b/extern/sector-storage/stores/remote.go @@ -518,13 +518,22 @@ func (r *Remote) CheckIsUnsealed(ctx context.Context, s storage.SectorRef, offse return false, xerrors.Errorf("has allocated: %w", err) } + // close the local unsealed file. if err := r.pfHandler.Close(pf); err != nil { return false, xerrors.Errorf("failed to close partial file: %s", err) } log.Debugf("checked if local partial file has the piece %s (+%d,%d), returning answer=%t", path, offset, size, has) - return has, nil + + // Sector files can technically not have a piece unsealed locally, but have it unsealed in remote storage, so we probably + // want to return only if has is true + if has { + return has, nil + } } + // --- We don't have the unsealed piece in an unsealed sector file locally + // Check if we have it in a remote cluster. + si, err := r.index.StorageFindSector(ctx, s.ID, ft, 0, false) if err != nil { return false, xerrors.Errorf("StorageFindSector: %s", err) @@ -601,19 +610,18 @@ func (r *Remote) Reader(ctx context.Context, s storage.SectorRef, offset, size a } log.Debugf("check if partial file is allocated %s (+%d,%d)", path, offset, size) - if !has { - log.Debugf("miner has unsealed file but not unseal piece, %s (+%d,%d)", path, offset, size) - if err := r.pfHandler.Close(pf); err != nil { - return nil, xerrors.Errorf("close partial file: %w", err) - } - return nil, nil + if has { + log.Infof("returning piece reader for local unsealed piece sector=%+v, (offset=%d, size=%d)", s.ID, offset, size) + return r.pfHandler.Reader(pf, storiface.PaddedByteIndex(offset), size) } - log.Infof("returning piece reader for local unsealed piece sector=%+v, (offset=%d, size=%d)", s.ID, offset, size) - return r.pfHandler.Reader(pf, storiface.PaddedByteIndex(offset), size) + log.Debugf("miner has unsealed file but not unseal piece, %s (+%d,%d)", path, offset, size) + if err := r.pfHandler.Close(pf); err != nil { + return nil, xerrors.Errorf("close partial file: %w", err) + } } - // --- We don't have the unsealed sector file locally + // --- We don't have the unsealed piece in an unsealed sector file locally // if we don't have the unsealed sector file locally, we'll first lookup the Miner Sector Store Index // to determine which workers have the unsealed file and then query those workers to know diff --git a/extern/sector-storage/stores/remote_test.go b/extern/sector-storage/stores/remote_test.go index b06f2605f..b708bb68f 100644 --- a/extern/sector-storage/stores/remote_test.go +++ b/extern/sector-storage/stores/remote_test.go @@ -152,7 +152,7 @@ func TestReader(t *testing.T) { }, // --- nil reader when local unsealed file does NOT have unsealed piece - "nil reader when local unsealed file does not have the piece": { + "nil reader when local unsealed file does not have the unsealed piece and remote sector also dosen't have the unsealed piece": { storeFnc: func(l *mocks.MockStore) { mockSectorAcquire(l, sectorRef, pfPath, nil) }, @@ -163,7 +163,20 @@ func TestReader(t *testing.T) { false, nil) pf.EXPECT().Close(emptyPartialFile).Return(nil).Times(1) + }, + + indexFnc: func(in *mocks.MockSectorIndex, url string) { + si := stores.SectorStorageInfo{ + URLs: []string{url}, + } + + in.EXPECT().StorageFindSector(gomock.Any(), sectorRef.ID, storiface.FTUnsealed, gomock.Any(), + false).Return([]stores.SectorStorageInfo{si}, nil).Times(1) + }, + + needHttpServer: true, + getAllocatedReturnCode: 500, }, // ---- nil reader when none of the remote unsealed file has unsealed piece @@ -232,6 +245,37 @@ func TestReader(t *testing.T) { }, // --- Success for remote unsealed file + // --- Success for remote unsealed file + "successfully fetches reader from remote unsealed piece when local unsealed file does NOT have the unsealed Piece": { + storeFnc: func(l *mocks.MockStore) { + mockSectorAcquire(l, sectorRef, pfPath, nil) + }, + + pfFunc: func(pf *mocks.MockpartialFileHandler) { + mockPartialFileOpen(pf, sectorSize, pfPath, nil) + mockCheckAllocation(pf, offset, size, emptyPartialFile, + false, nil) + + pf.EXPECT().Close(emptyPartialFile).Return(nil).Times(1) + + }, + + indexFnc: func(in *mocks.MockSectorIndex, url string) { + si := stores.SectorStorageInfo{ + URLs: []string{url}, + } + + in.EXPECT().StorageFindSector(gomock.Any(), sectorRef.ID, storiface.FTUnsealed, gomock.Any(), + false).Return([]stores.SectorStorageInfo{si}, nil).Times(1) + }, + + needHttpServer: true, + getSectorReturnCode: 200, + getAllocatedReturnCode: 200, + expectedSectorBytes: bz, + expectedNonNilReader: true, + }, + "successfully fetches reader for piece from remote unsealed piece": { storeFnc: func(l *mocks.MockStore) { mockSectorAcquire(l, sectorRef, "", nil) @@ -439,7 +483,7 @@ func TestCheckIsUnsealed(t *testing.T) { }, // false when local unsealed file does NOT have unsealed piece - "false when local unsealed file does not have the piece": { + "false when local unsealed file does not have the piece and remote sector too dosen't have the piece": { storeFnc: func(l *mocks.MockStore) { mockSectorAcquire(l, sectorRef, pfPath, nil) }, @@ -451,6 +495,18 @@ func TestCheckIsUnsealed(t *testing.T) { pf.EXPECT().Close(emptyPartialFile).Return(nil).Times(1) }, + + indexFnc: func(in *mocks.MockSectorIndex, url string) { + si := stores.SectorStorageInfo{ + URLs: []string{url}, + } + + in.EXPECT().StorageFindSector(gomock.Any(), sectorRef.ID, storiface.FTUnsealed, gomock.Any(), + false).Return([]stores.SectorStorageInfo{si}, nil).Times(1) + }, + + needHttpServer: true, + getAllocatedReturnCode: 500, }, "false when none of the worker has the unsealed piece": { @@ -489,7 +545,7 @@ func TestCheckIsUnsealed(t *testing.T) { }, // --- Success for remote unsealed file - "successfully fetches reader for piece from remote unsealed piece": { + "true if we have a remote unsealed piece": { storeFnc: func(l *mocks.MockStore) { mockSectorAcquire(l, sectorRef, "", nil) }, @@ -507,6 +563,33 @@ func TestCheckIsUnsealed(t *testing.T) { getAllocatedReturnCode: 200, expectedIsUnealed: true, }, + + "true when local unsealed file does NOT have the unsealed Piece but remote sector has the unsealed piece": { + storeFnc: func(l *mocks.MockStore) { + mockSectorAcquire(l, sectorRef, pfPath, nil) + }, + + pfFunc: func(pf *mocks.MockpartialFileHandler) { + mockPartialFileOpen(pf, sectorSize, pfPath, nil) + mockCheckAllocation(pf, offset, size, emptyPartialFile, + false, nil) + + pf.EXPECT().Close(emptyPartialFile).Return(nil).Times(1) + }, + + indexFnc: func(in *mocks.MockSectorIndex, url string) { + si := stores.SectorStorageInfo{ + URLs: []string{url}, + } + + in.EXPECT().StorageFindSector(gomock.Any(), sectorRef.ID, storiface.FTUnsealed, gomock.Any(), + false).Return([]stores.SectorStorageInfo{si}, nil).Times(1) + }, + + needHttpServer: true, + getAllocatedReturnCode: 200, + expectedIsUnealed: true, + }, } for name, tc := range tcs {