Skip to content

Commit

Permalink
fix: tracer to track delivered message if duplicate (#385)
Browse files Browse the repository at this point in the history
* feat: tracer to track delivered message if duplicate

* chore: add iwantPromiseResolvedFromDuplicate metric
  • Loading branch information
twoeths authored Dec 22, 2022
1 parent dbeb879 commit 0c8ddee
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 1 deletion.
4 changes: 4 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,10 @@ export class GossipSub extends EventEmitter<GossipsubEvents> implements PubSub<G
case MessageStatus.duplicate:
// Report the duplicate
this.score.duplicateMessage(from.toString(), validationResult.msgIdStr, rpcMsg.topic)
// due to the collision of fastMsgIdFn, 2 different messages may end up the same fastMsgId
// so we need to also mark the duplicate message as delivered or the promise is not resolved
// and peer gets penalized. See https://github.com/ChainSafe/js-libp2p-gossipsub/pull/385
this.gossipTracer.deliverMessage(validationResult.msgIdStr, true)
this.mcache.observeDuplicate(validationResult.msgIdStr, from.toString())
return

Expand Down
5 changes: 5 additions & 0 deletions src/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,11 @@ export function getMetrics(
name: 'gossipsub_iwant_promise_resolved_total',
help: 'Total count of resolved IWANT promises'
}),
/** Total count of resolved IWANT promises from duplicate messages */
iwantPromiseResolvedFromDuplicate: register.gauge({
name: 'gossipsub_iwant_promise_resolved_from_duplicate_total',
help: 'Total count of resolved IWANT promises from duplicate messages'
}),
/** Total count of peers we have asked IWANT promises that are resolved */
iwantPromiseResolvedPeers: register.gauge({
name: 'gossipsub_iwant_promise_resolved_peers',
Expand Down
3 changes: 2 additions & 1 deletion src/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class IWantTracer {
/**
* Someone delivered a message, stop tracking promises for it
*/
deliverMessage(msgIdStr: MsgIdStr): void {
deliverMessage(msgIdStr: MsgIdStr, isDuplicate = false): void {
this.trackMessage(msgIdStr)

const expireByPeer = this.promises.get(msgIdStr)
Expand All @@ -116,6 +116,7 @@ export class IWantTracer {

if (this.metrics) {
this.metrics.iwantPromiseResolved.inc(1)
if (isDuplicate) this.metrics.iwantPromiseResolvedFromDuplicate.inc(1)
this.metrics.iwantPromiseResolvedPeers.inc(expireByPeer.size)
}
}
Expand Down

0 comments on commit 0c8ddee

Please sign in to comment.