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.

Sign up for GitHub

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

feat: Make add piece idempotent #8160

Merged
merged 6 commits into from
Mar 2, 2022
Merged

feat: Make add piece idempotent #8160

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26707a5
make add piece idempotent
aarshkshah1992 Feb 21, 2022
1245871
guard with ctx
aarshkshah1992 Feb 21, 2022
c2522e8
idempotent add piece
aarshkshah1992 Feb 21, 2022
56926b3
changes as per review
aarshkshah1992 Feb 21, 2022
8e62fec
address review
aarshkshah1992 Feb 22, 2022
f0f2b3e
changes as per review
aarshkshah1992 Feb 22, 2022
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
42 changes: 23 additions & 19 deletions extern/storage-sealing/input.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,41 +311,45 @@ func (m *Sealing) SectorAddPieceToAny(ctx context.Context, size abi.UnpaddedPiec
}

m.inputLk.Lock()
if _, exist := m.pendingPieces[proposalCID(deal)]; exist {
if pp, exist := m.pendingPieces[proposalCID(deal)]; exist {
m.inputLk.Unlock()
return api.SectorOffset{}, xerrors.Errorf("piece for deal %s already pending", proposalCID(deal))
select {
case <-pp.doneCh:
res := pp.resp.Load().(*pieceAcceptResp)
return api.SectorOffset{Sector: res.sn, Offset: res.offset.Padded()}, res.err
case <-ctx.Done():
return api.SectorOffset{}, ctx.Err()
}
}

resCh := make(chan struct {
sn abi.SectorNumber
offset abi.UnpaddedPieceSize
err error
}, 1)

m.pendingPieces[proposalCID(deal)] = &pendingPiece{
doneCh := make(chan struct{}, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
doneCh := make(chan struct{}, 1)
doneCh := make(chan struct{})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this for safety, dosen't hurt because the code that writes to this is called sync by the sealer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never really write to this channel, just close it, which doesn't block.

pp := &pendingPiece{
doneCh: doneCh,
size: size,
deal: deal,
data: data,
assigned: false,
accepted: func(sn abi.SectorNumber, offset abi.UnpaddedPieceSize, err error) {
resCh <- struct {
sn abi.SectorNumber
offset abi.UnpaddedPieceSize
err error
}{sn: sn, offset: offset, err: err}
},
}
pp.accepted = func(sn abi.SectorNumber, offset abi.UnpaddedPieceSize, err error) {
pp.resp.Store(&pieceAcceptResp{sn, offset, err})
close(pp.doneCh)
}

m.pendingPieces[proposalCID(deal)] = pp
go func() {
defer m.inputLk.Unlock()
if err := m.updateInput(ctx, sp); err != nil {
log.Errorf("%+v", err)
}
}()

res := <-resCh

return api.SectorOffset{Sector: res.sn, Offset: res.offset.Padded()}, res.err
select {
case <-doneCh:
res := pp.resp.Load().(*pieceAcceptResp)
return api.SectorOffset{Sector: res.sn, Offset: res.offset.Padded()}, res.err
case <-ctx.Done():
return api.SectorOffset{}, ctx.Err()
}
}

func (m *Sealing) MatchPendingPiecesToOpenSectors(ctx context.Context) error {
Expand Down
10 changes: 10 additions & 0 deletions extern/storage-sealing/sealing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"sync"
"sync/atomic"
"time"

"github.com/ipfs/go-cid"
Expand Down Expand Up @@ -140,7 +141,16 @@ func (o *openSector) dealFitsInLifetime(dealEnd abi.ChainEpoch, expF func(sn abi
return expiration >= dealEnd, nil
}

type pieceAcceptResp struct {
sn abi.SectorNumber
offset abi.UnpaddedPieceSize
err error
}

type pendingPiece struct {
doneCh chan struct{}
resp atomic.Value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think resp needs to be an atomic value - the doneCh prevents concurrent access

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. fixed.


size abi.UnpaddedPieceSize
deal api.PieceDealInfo

Expand Down