-
Notifications
You must be signed in to change notification settings - Fork 112
Conversation
@aschmahmann this is ready for review. passes on each OS once. the failing tests are all documented flaky. |
dupMetric: dupHist, | ||
allMetric: allHist, | ||
sentHistogram: sentHistogram, | ||
sendTimeHistogram: sendTimeHistogram, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still useful? If it's not documented then it's not useful and we should remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is useful to know if your outbound connection is slow. Let's keep it. Where is the right place to document it? The metric itself is documented when initialized, so the doc shows up on the grafana dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, that seems fine. We should try and track this internally on our infra (and ask Pinata to try on theirs) so we can get an understanding of what these numbers look like and if the time resolution is anywhere close to correct.
@aschmahmann ready for another pass. |
1. Minimum timeout of 10s. 2. We add 2s due to latencies. 3. Minimum bandwidth of 100kbit/s. 4. Maximum message send time of 2M (way more time than necessary).
5d0a433
to
d4a71a1
Compare
[x] prometheus metric for how long it takes to send messages
[x] option for maximum outstanding bytes per peer
[x] tighter send timeouts
- Minimum timeout of 10s.
- We add 2s due to latencies.
- Minimum bandwidth of 100kbit/s.
- Maximum message send time of 2M (way more time than necessary).
[x] option for number of task workers
[x] option for number of engine task workers
Depends on: https://github.com/ipfs/go-peertaskqueue/pull/10/commits
Part of ipfs/kubo#8233