Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

Total wants gauge #402

Merged
merged 4 commits into from
Jun 2, 2020
Merged

Total wants gauge #402

merged 4 commits into from
Jun 2, 2020

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented May 29, 2020

Fixes #333

Currently we track the number of pending want-blocks in wantlist_total.

In this PR

  • wantlist_total is the total number of pending want-blocks + want-haves
  • want_blocks_total is the total number of pending want-blocks

@dirkmc dirkmc requested a review from Stebalien June 1, 2020 16:22
@Stebalien
Copy link
Member

Can we track number of wants instead of number of wants per peer? I'm looking to see "how many different things are we trying to download". This gives us a good metric on how "loaded" a machine is.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

nit but otherwise lgtm.

@@ -207,6 +238,9 @@ func (pwm *peerWantManager) sendCancels(cancelKs []cid.Cid) {
}
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to simplify all of this to:

(and remove the sets)

if pwm.broadcastWants.Has(c) {
    broadcastCancels = append(broadcastCancels, c)
    pwm.wantGague.Dec()
} else if _, ok := pwm.wantPeers[c]; ok {
    pwm.wantGague.Dec()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that we need to decrement the want-block gauge as well as the all-wants gauge, and we don't know if pwm.wantPeers[c] points to a want-have or a want-block.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Eh, then we can profile later to see if it makes a difference.

@dirkmc dirkmc merged commit 88373cd into master Jun 2, 2020
@dirkmc dirkmc deleted the feat/want-gauge branch June 2, 2020 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Want gauge should match ipfs bitswap wantlist | wc -l
2 participants