Skip to content

AddrSplosion -- Partie Deux Avec Vengeance - #672

Merged
jbenet merged 5 commits into
masterfrom
clear-addrs
Jan 27, 2015
Merged

AddrSplosion -- Partie Deux Avec Vengeance#672
jbenet merged 5 commits into
masterfrom
clear-addrs

Conversation

@jbenet

@jbenet jbenet commented Jan 27, 2015

Copy link
Copy Markdown
Member

This PR reduces the effects of addr-splosion.

  • Nodes clear their own internal cache when they
    receive ground truth from the peer itself.
  • Nodes' address cache expires. Each addr has a
    TTL (for now set to 1hr). After which nodes
    must resolve addrs again. (this may be too
    aggressive and can be revisited once we bring
    in proper addr management)

Related to: #573, #603

@jbenet jbenet added the status/in-progress In progress label Jan 27, 2015
Comment thread p2p/peer/peerstore.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jbenet need to hold write lock to delete these?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep, it is. defered. github's omitting it from the diff. (but would've been a great catch)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant that the lock is currently held for read access.

ab.RLock()
defer ab.RUnlock()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but needs to be Lock() Unlock() since there exist map deletions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ahhhh great catch. right you are. thanks

jbenet added a commit that referenced this pull request Jan 27, 2015
AddrSplosion -- Partie Deux Avec Vengeance
@jbenet
jbenet merged commit b16adfe into master Jan 27, 2015
@jbenet jbenet removed the status/in-progress In progress label Jan 27, 2015
@jbenet
jbenet deleted the clear-addrs branch January 27, 2015 07:18
@btc

btc commented Jan 27, 2015

Copy link
Copy Markdown
Contributor

LGTM

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.

2 participants