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

feat(shwap/bitswap): use new option for optimized Has check #3813

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
4 changes: 4 additions & 0 deletions share/shwap/p2p/bitswap/bitswap.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ const (
// We set it to be equal to targetMessageSize * N, so there can max N messages being prepared for
// a peer at once.
outstandingBytesPerPeer = targetMessageSize * 4
// replaceHasWithBlockMaxSize configures Bitswap to use Has method instead of GetSize to check existence
// of a CID in Blockstore.
replaceHasWithBlockMaxSize = 0
)

// NewNetwork constructs Bitswap network for Shwap protocol composition.
Expand Down Expand Up @@ -114,6 +117,7 @@ func NewServer(
server.MaxQueuedWantlistEntriesPerPeer(maxServerWantListsPerPeer),
server.WithTargetMessageSize(targetMessageSize),
server.MaxOutstandingBytesPerPeer(outstandingBytesPerPeer),
server.WithWantHaveReplaceSize(replaceHasWithBlockMaxSize),
}
return server.New(ctx, net, bstore, opts...)
}
Expand Down
4 changes: 4 additions & 0 deletions share/shwap/p2p/bitswap/block_fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ func (t testAccessorGetter) GetByHeight(context.Context, uint64) (eds.AccessorSt
return t.AccessorStreamer, nil
}

func (t testAccessorGetter) HasByHeight(context.Context, uint64) (bool, error) {
return true, nil
}

type testFetcher struct {
Fetched int

Expand Down
16 changes: 11 additions & 5 deletions share/shwap/p2p/bitswap/block_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
type AccessorGetter interface {
// GetByHeight returns an Accessor by its height.
GetByHeight(ctx context.Context, height uint64) (eds.AccessorStreamer, error)
// HasByHeight reports whether an Accessor for the height exists.
HasByHeight(ctx context.Context, height uint64) (bool, error)
}

// Blockstore implements generalized Bitswap compatible storage over Shwap containers
Expand Down Expand Up @@ -63,10 +65,9 @@ func (b *Blockstore) Get(ctx context.Context, cid cid.Cid) (blocks.Block, error)
}

func (b *Blockstore) GetSize(ctx context.Context, cid cid.Cid) (int, error) {
// TODO(@Wondertan): There must be a way to derive size without reading, proving, serializing and
// allocating Sample's block.Block or we could do hashing
// NOTE:Bitswap uses GetSize also to determine if we have content stored or not
// so simply returning constant size is not an option
// TODO(@Wondertan): Bitswap checks the size of the data(GetSize) before serving it via Get. This means
// GetSize may do an unnecessary read from disk which we can avoid by either caching on Blockstore level
// or returning constant size(we know at that point that we have requested data)
blk, err := b.Get(ctx, cid)
if err != nil {
return 0, err
Expand All @@ -75,10 +76,15 @@ func (b *Blockstore) GetSize(ctx context.Context, cid cid.Cid) (int, error) {
}

func (b *Blockstore) Has(ctx context.Context, cid cid.Cid) (bool, error) {
_, err := b.Get(ctx, cid)
blk, err := EmptyBlock(cid)
if err != nil {
return false, err
}

_, err = b.Getter.HasByHeight(ctx, blk.Height())
if err != nil {
return false, fmt.Errorf("checking EDS Accessor for height %v: %w", blk.Height(), err)
}
return true, nil
}

Expand Down
Loading