Skip to content

Conversation

@yacovm
Copy link
Contributor

@yacovm yacovm commented Jul 14, 2025

Why this should be merged

It can be insightful to measure how effective and efficient is our gossip pull mechanism that is based on bloom filters.

How this works

This commit adds a metric to measure the bloom filter hit rate % for the gossip pull queries.

How this was tested

  • Updated TestGossiperGossip accordingly to reflect the metrics
  • Ran a modified avalanchego node and installed prometheus and grafana and observed the following:
image

Need to be documented in RELEASES.md?

No.

Copilot AI review requested due to automatic review settings July 14, 2025 23:03
@yacovm yacovm requested a review from joshua-kim as a code owner July 14, 2025 23:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds instrumentation to measure the effectiveness of the bloom filter used in the gossip pull mechanism.

  • Track and compute bloom filter hits and misses in AppRequest
  • Introduce a bloomFilterHitRate histogram in the metrics and register it
  • Extend unit tests to cover the new histogram and the computeBloomFilterHitPercentage function

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
network/p2p/gossip/handler.go Count filter hits/misses and compute hit percentage
network/p2p/gossip/gossip.go Add bloomFilterHitRate histogram and register it
network/p2p/gossip/gossip_test.go Update tests to assert metric observation and new logic
Comments suppressed due to low confidence (3)

network/p2p/gossip/gossip.go:115

  • [nitpick] Metric name bloomfilter_hit_rate is inconsistent with the project’s snake_case convention; consider renaming to bloom_filter_hit_rate or prefixing with gossip_ to match other metric names.
			Name:      "bloomfilter_hit_rate",

network/p2p/gossip/gossip_test.go:633

  • [nitpick] Consider adding tests for the error branches in computeBloomFilterHitPercentage (e.g., when safemath.Add or Mul overflows) to verify that ok is false in those cases.
func TestComputeBloomFilterHitPercentage(t *testing.T) {

network/p2p/gossip/handler.go:18

  • The computeBloomFilterHitPercentage function uses zap.Uint64 and zap.Error but go.uber.org/zap is not imported, causing a compile error. Add import "go.uber.org/zap" or switch to the existing logging API.
	safemath "github.com/ava-labs/avalanchego/utils/math"

This commit adds a metric to measure the bloom filter hit rate % for the gossip pull queries.

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@yacovm yacovm marked this pull request as draft July 15, 2025 16:13
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@yacovm yacovm marked this pull request as ready for review July 15, 2025 18:36
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 18, 2025
Merged via the queue into ava-labs:master with commit e2bcbaf Jul 18, 2025
29 checks passed
@github-project-automation github-project-automation bot moved this to Done 🎉 in avalanchego Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants