Skip to content

Fix minMeshMessageDeliveriesWindow #218

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

Merged
merged 4 commits into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions ts/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,8 @@ export const ACCEPT_FROM_WHITELIST_MAX_MESSAGES = 128
* this peer up to this time duration.
*/
export const ACCEPT_FROM_WHITELIST_DURATION_MS = 1000

/**
* The default MeshMessageDeliveriesWindow to be used in metrics.
*/
export const DEFAULT_METRIC_MESH_MESSAGE_DELIVERIES_WINDOWS = 1000
16 changes: 10 additions & 6 deletions ts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,18 @@ export default class Gossipsub extends EventEmitter {
throw Error('Must set metricsTopicStrToLabel with metrics')
}

// in theory, each topic has its own meshMessageDeliveriesWindow param
// however in lodestar, we configure it mostly the same so just pick the max of positive ones
// (some topics have meshMessageDeliveriesWindow as 0)
const maxMeshMessageDeliveriesWindow = Math.max(
...Object.values(opts.scoreParams.topics).map((topicParam) => topicParam.meshMessageDeliveriesWindow),
constants.DEFAULT_METRIC_MESH_MESSAGE_DELIVERIES_WINDOWS
)

const metrics = getMetrics(options.metricsRegister, options.metricsTopicStrToLabel, {
gossipPromiseExpireSec: constants.GossipsubIWantFollowupTime / 1000,
gossipPromiseExpireSec: this.opts.gossipsubIWantFollowupTime / 1000,
behaviourPenaltyThreshold: opts.scoreParams.behaviourPenaltyThreshold,
// in theory, each topic has its own meshMessageDeliveriesWindow param
// however in lodestar, we configure it the same so just pick the min one
minMeshMessageDeliveriesWindow: Math.min(
...Object.values(opts.scoreParams.topics).map((topicParam) => topicParam.meshMessageDeliveriesWindow)
)
maxMeshMessageDeliveriesWindow
})

metrics.mcacheSize.addCollect(() => this.onScrapeMetrics(metrics))
Expand Down
12 changes: 6 additions & 6 deletions ts/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export type Metrics = ReturnType<typeof getMetrics>
export function getMetrics(
register: MetricsRegister,
topicStrToLabel: TopicStrToLabel,
opts: { gossipPromiseExpireSec: number; behaviourPenaltyThreshold: number; minMeshMessageDeliveriesWindow: number }
opts: { gossipPromiseExpireSec: number; behaviourPenaltyThreshold: number; maxMeshMessageDeliveriesWindow: number }
) {
// Using function style instead of class to prevent having to re-declare all MetricsPrometheus types.

Expand Down Expand Up @@ -346,11 +346,11 @@ export function getMetrics(
help: 'Time since the 1st duplicated message validated',
labelNames: ['topic'],
buckets: [
0.25 * opts.minMeshMessageDeliveriesWindow,
0.5 * opts.minMeshMessageDeliveriesWindow,
1 * opts.minMeshMessageDeliveriesWindow,
2 * opts.minMeshMessageDeliveriesWindow,
4 * opts.minMeshMessageDeliveriesWindow
0.25 * opts.maxMeshMessageDeliveriesWindow,
0.5 * opts.maxMeshMessageDeliveriesWindow,
1 * opts.maxMeshMessageDeliveriesWindow,
2 * opts.maxMeshMessageDeliveriesWindow,
4 * opts.maxMeshMessageDeliveriesWindow
]
}),
/** Total count of late msg delivery total by topic */
Expand Down
4 changes: 3 additions & 1 deletion ts/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ export class IWantTracer {
const maxMs = Date.now() - this.requestMsByMsgExpire

for (const [k, v] of this.requestMsByMsg.entries()) {
if (v < maxMs) {
if (v >= maxMs) {
// messages that stay too long in the requestMsByMsg map, delete
this.requestMsByMsg.delete(k)
} else {
// recent messages, keep them
// sort by insertion order
break
}
Expand Down