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

Increase resolution of delay metrics #217

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

dapplion
Copy link
Contributor

Add more buckets in parameters we have to fine tune to have sufficient data to choose the right values.

For duplicateMsgDeliveryDelay remove the topic label to reduce total number of metrics and use a simple counter duplicateMsgLateDelivery by topic instead.

@dapplion dapplion requested a review from twoeths March 24, 2022 03:00
@codecov-commenter
Copy link

Codecov Report

Merging #217 (10eae93) into dapplion/gossipsub (ca7733f) will decrease coverage by 0.36%.
The diff coverage is 13.63%.

@@                  Coverage Diff                   @@
##           dapplion/gossipsub     #217      +/-   ##
======================================================
- Coverage               79.28%   78.91%   -0.37%     
======================================================
  Files                      24       24              
  Lines                    7671     7685      +14     
  Branches                  771      765       -6     
======================================================
- Hits                     6082     6065      -17     
- Misses                   1589     1620      +31     
Impacted Files Coverage Δ
ts/metrics.ts 22.85% <0.00%> (-0.44%) ⬇️
ts/score/peer-score.ts 92.09% <100.00%> (-0.49%) ⬇️
ts/message-cache.ts 84.11% <0.00%> (-1.18%) ⬇️
ts/index.ts 87.39% <0.00%> (-0.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca7733f...10eae93. Read the comment docs.

@twoeths twoeths merged commit 6b99dbf into dapplion/gossipsub Mar 24, 2022
@twoeths twoeths deleted the dapplion/onDuplicateMsgDelivery branch March 24, 2022 03:11
twoeths added a commit that referenced this pull request Apr 5, 2022
* Add metrics and revamp logic

Declare prometheus metrics in gossipsub

Add missing metric labels

Fix metrics types

Add onReportValidation

* Compile tests

* Fix tests

* Customize await policy

* Run prettier on files

* Relax heartbeat test condition

* Re-add util package for browser testing

* Remove unused dependencies

* Add getMeshPeers

* Fix rpc metric name

* Improve metrics

* More gossip promise metrics

* Fix metrics typo

* Clarify metrics names

* Track metric of score deltas

* Fix score metrics

* Add buckets for gossipsub_score_cache_delta

* Review PeerScore logic

* Dump peer stats

* Fix PeerStats constructor

* Enable strict typescript checks

* Fix compute-score logic

* f

* Fix peer stats types

* Reenable go-gossipsub tests (#201)

* Add checkReceivedSubscriptions

* Add checkReceivedSubscriptions to 'test gossipsub multihops'

* Add checkReceivedSubscriptions to 'test gossipsub tree topology'

* test gossipsub star topology with signed peer records

* Fix 'test gossipsub direct peers'

* Fix 'test gossipsub flood publish'

* Fix 'test gossipsub star topology with signed peer records'

* 'direct peers' test: wait for subscriptions event again

* 'direct peer': await for 2 peer:connect events

* 'direct peers': add missing Promise.all

* Expect topic peers to contain peer id

* Fix test types

* Set as connected in addPeer()

* Prune publishedMessageIds

* Fix markFirstMessageDelivery typo

* Same logic in scoreMetrics

* More metrics for p3 and p7 (#213)

* Add behaviourPenalty metrics

* Add duplicateMsgDelivery metric

* Observe topic and peer in duplicateMsgDelivery topic

* Remove peerId from duplicateMsgDelivery metric

* Use min meshMessageDeliveriesWindow

* Use topic label for duplicateMsgDelivery metric

* Record duplicateMsgDelivery in all cases

* Forward messages to floodsub peers (#214)

* Forward messages to floodsub peers

* Add comments

Co-authored-by: Lion - dapplion <[email protected]>

* Add missing msgId in validateReceivedMessage (#215)

* Fix 'test gossipsub opportunistic grafting'

* Fix tests in browser

* Fix tests suspended issue in browsers

* Fix 'test gossipsub fanout expiry' go-gossipsub test

* GossipsubIWantFollowupTime as a param (#216)

* Increase resolution of delay metrics (#217)

* Fix minMeshMessageDeliveriesWindow (#218)

* Fix minMeshMessageDeliveriesWindow

* Fix gossipsubIWantFollowupTime for metric

* Fix tracer prune()

* Change to maxMeshMessageDeliveriesWindow

* Use maxMeshMessageDeliveriesWindowSec for metric

* Rename gossipsubIWantFollowupMs option, revert/correct tracer.prune() logic

* Reset behaviourPenalty histogram to track current count

* publish(): return number of sent peers

* Add getScore()

* makePrune: update PeerStat so that we don't apply p3 penalty

* Remove redundant this.score.prune() in heartbeat

* Track iasked cache size per heartbeat and remove TODOs

* validateReceivedMessage: check duplicate message first (#223)

Co-authored-by: tuyennhv <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants