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

set createTime per node and purge old nodes if maxNodes is reached #57

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 5 additions & 1 deletion dht.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func New(config *Config) (node *DHT, err error) {
node = &DHT{
config: cfg,
routingTable: newRoutingTable(),
peerStore: newPeerStore(cfg.MaxInfoHashes, cfg.MaxInfoHashPeers),
peerStore: newPeerStore(cfg.MaxInfoHashes, cfg.MaxInfoHashPeers, cfg.MaxNodes),
PeersRequestResults: make(chan map[InfoHash][]string, 1),
stop: make(chan bool),
exploredNeighborhood: false,
Expand Down Expand Up @@ -512,6 +512,10 @@ func (d *DHT) needMoreNodes() bool {
return n < minNodes || n*2 < d.config.MaxNodes
}

func (d *DHT) GetNumNodes() int {
return d.routingTable.numNodes()
Copy link
Owner

Choose a reason for hiding this comment

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

Do we really need this? I don't think we do and I think it's better to only expose methods when we really need to. Besides, I think this isn't safe to be used concurrently while the DHT is running?

Copy link
Author

Choose a reason for hiding this comment

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

agree. was just usefull for my debuging.

}

func (d *DHT) needMorePeers(ih InfoHash) bool {
return d.peerStore.alive(ih) < d.config.NumTargetPeers
}
Expand Down
2 changes: 2 additions & 0 deletions krpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type remoteNode struct {
pendingQueries map[string]*queryType // key: transaction ID
pastQueries map[string]*queryType // key: transaction ID
reachable bool
createTime time.Time
lastResponseTime time.Time
lastSearchTime time.Time
ActiveDownloads []string // List of infohashes we know this peer is downloading.
Expand All @@ -44,6 +45,7 @@ func newRemoteNode(addr net.UDPAddr, id string) *remoteNode {
lastQueryID: newTransactionId(),
id: id,
reachable: false,
createTime: time.Now(),
pendingQueries: map[string]*queryType{},
pastQueries: map[string]*queryType{},
}
Expand Down
4 changes: 3 additions & 1 deletion peer_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,13 @@ func (p *peerContactsSet) Alive() int {
return ret
}

func newPeerStore(maxInfoHashes, maxInfoHashPeers int) *peerStore {
func newPeerStore(maxInfoHashes, maxInfoHashPeers, maxNodes int) *peerStore {
return &peerStore{
infoHashPeers: lru.New(maxInfoHashes),
localActiveDownloads: make(map[InfoHash]bool),
maxInfoHashes: maxInfoHashes,
maxInfoHashPeers: maxInfoHashPeers,
maxNodes: maxNodes,
}
}

Expand All @@ -141,6 +142,7 @@ type peerStore struct {
localActiveDownloads map[InfoHash]bool
maxInfoHashes int
maxInfoHashPeers int
maxNodes int
}

func (h *peerStore) get(ih InfoHash) *peerContactsSet {
Expand Down
6 changes: 6 additions & 0 deletions routing_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,12 @@ func (r *routingTable) cleanup(cleanupPeriod time.Duration, p *peerStore) (needP
r.kill(n, p)
continue
}
// kill old and currently unused nodes if nodeCount is > maxNodes
if len(r.addresses) > p.maxNodes && time.Since(n.createTime) > cleanupPeriod && len(n.pendingQueries) == 0 {
Copy link
Owner

Choose a reason for hiding this comment

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

I'd say we should kill the node even if there are pending queries. If it's so old, better refresh the routing table with newer models?

Copy link
Author

Choose a reason for hiding this comment

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

we may have just sent out a query a second ago to that specific node, because of the nearest distance to the searched hash.. so we don't want to loose that result?

log.V(4).Infof("DHT: Old node with 0 pendingQueries. Deleting")
r.kill(n, p)
continue
}
if n.reachable {
if len(n.pendingQueries) == 0 {
goto PING
Expand Down