Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

api, metrics, network: check caps when deciding on next peer for a chunk #1749

Merged
merged 6 commits into from
Sep 12, 2019
Merged

api, metrics, network: check caps when deciding on next peer for a chunk #1749

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7ef600f
api, metrics, network: check caps when deciding on next peer for a chunk
nonsense Sep 12, 2019
b278c14
network: WARN for a global chunk timeout
nonsense Sep 12, 2019
c1de0f4
network: remove circular dep between retrieve and stream
nonsense Sep 12, 2019
dcdbc3d
network: do not explicitly check for light nodes
nonsense Sep 12, 2019
8b5ff92
network: use cap for `bzz-retrieve` and not `bzz-stream`
nonsense Sep 12, 2019
ecf0094
network: revert back check for light nodes
nonsense Sep 12, 2019
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
2 changes: 1 addition & 1 deletion api/inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (i *Inspector) DeliveriesPerPeer() map[string]int64 {
// iterate connection in kademlia
i.hive.Kademlia.EachConn(nil, 255, func(p *network.Peer, po int) bool {
// get how many chunks we receive for retrieve requests per peer
peermetric := fmt.Sprintf("chunk.delivery.%x", p.Over()[:16])
peermetric := fmt.Sprintf("network.retrieve.chunk.delivery.%x", p.Over()[:16])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Independent of the findPeer - RequestFromPeers bug, but nonetheless a nice-to-have fix, so that DeliveriesFromPeers debug API works.


res[fmt.Sprintf("%x", p.Over()[:16])] = metrics.GetOrRegisterCounter(peermetric, nil).Count()

Expand Down
2 changes: 1 addition & 1 deletion metrics/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func datadirDiskUsage(path string, d time.Duration) {
for range time.Tick(d) {
bytes, err := dirSize(path)
if err != nil {
log.Warn("cannot get disk space", "err", err)
log.Trace("cannot get disk space", "err", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This warning is annoying when we hit LevelDB during compaction, and it is mostly useless in our monitoring system, therefore I am reducing its level.

}

metrics.GetOrRegisterGauge("datadir.usage", nil).Update(bytes)
Expand Down
5 changes: 5 additions & 0 deletions network/retrieval/retrieve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethersphere/swarm/chunk"
"github.com/ethersphere/swarm/network"
"github.com/ethersphere/swarm/network/stream/v2"
Copy link
Member

Choose a reason for hiding this comment

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

Actually, there is a circular dependency in tests reported by linter. Retrieval is needed in stream tests, and here stream is imported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, so I will just hard-code the name of the protocol. This is not something that should change often. OK?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is OK.

"github.com/ethersphere/swarm/network/timeouts"
"github.com/ethersphere/swarm/p2p/protocols"
"github.com/ethersphere/swarm/spancontext"
Expand Down Expand Up @@ -231,6 +232,10 @@ func (r *Retrieval) findPeer(ctx context.Context, req *storage.Request) (retPeer
return true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the p.HasCap change, I think we don't need this if p.LightNode check, @janos what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this can be removed. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted it back, because we change the check for capabilities for bzz-retrieve, and not bzz-stream.

}

if !p.HasCap(stream.Spec.Name) {
return true
}

// do not send request back to peer who asked us. maybe merge with SkipPeer at some point
if bytes.Equal(req.Origin.Bytes(), id.Bytes()) {
return true
Expand Down