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

fix(pubsub): Switch to a last-seen cache strategy #12389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Stebalien
Copy link
Member

Proposed Changes

We were using the default first-seen strategy which means we'd loop messages every 2 minutes (when they expire from the cache). This new strategy keeps the message in the cache until we haven't seen it for 2 minutes, ensuring that it eventually drops off the network (with high likelihood).

The downside to this change, and a reason to potentially not include it, is that it may significantly harm message propegation. Specifically:

  1. Node sends a message.
  2. Node's peers propegate it.
  3. Peer re-sends it.
  4. Nodes peers (the same peers) have already seen the message so they don't propagate it. But some other nodes on the network didn't get it, so the message is censored.

In practice, we've configured our re-broadcast interval to be greater than the cache time so this may not be an issue? But this still isn't a straight-forward merge.

Additional Info

The correct fix is message expiration: libp2p/go-libp2p-pubsub#573, but that requires deeper protocol changes.

Checklist

Before you mark the PR ready for review, please make sure that:

We were using the default first-seen strategy which means we'd loop
messages every 2 minutes (when they expire from the cache). This new
strategy keeps the message in the cache until we haven't seen it for 2
minutes, ensuring that it eventually drops off the network (with high
likelihood).

The correct fix is message expiration:
libp2p/go-libp2p-pubsub#573, but that requires
deeper protocol changes.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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.

1 participant