Cache peer score #175
Cache peer score #175
Conversation
twoeths
force-pushed
the
tuyen/peer-score-performance
branch
from
56da592 to
6716d26
Compare
Codecov Report
@@ Coverage Diff @@
## master #175 +/- ##
=======================================
Coverage 76.58% 76.58%
=======================================
Files 2 2
Lines 1905 1905
Branches 142 140 -2
=======================================
Hits 1459 1459
Misses 446 446 Continue to review full report at Codecov.
|
| * | ||
| * @type {Map<string, AcceptFromWhiteListEntry} | ||
| */ | ||
| this.acceptFromWhitelist = new Map() |
There was a problem hiding this comment.
I think we need a place where these entries get routinely pruned so they don't build up.
Maybe you can prune old entries on every N heartbeats
There was a problem hiding this comment.
also remove the peer's entry in _removePeer
There was a problem hiding this comment.
@wemeetagain so I removed it in _removePeer but regarding this
I think we need a place where these entries get routinely pruned so they don't build up.
Maybe you can prune old entries on every N heartbeats
I don't think it's necessary as we only do the prune in _removePeer() function for other map as well (unless there's already a leak somewhere, we'll have to do the prune regularly for all of the map). In acceptFrom() we also do the prune if the score is negative
twoeths
force-pushed
the
tuyen/peer-score-performance
branch
from
631b466 to
17c8976
Compare
ts/score/peer-score.ts
Outdated
| /** | ||
| * The time after which the cached score for a peer is no longer valid. | ||
| */ | ||
| scoreCacheUntil: Map<string, number> |
There was a problem hiding this comment.
Clarify exactly what number is: unix timestamp in miliseconds
There was a problem hiding this comment.
Consider merging scoreCache and scoreCacheUntil, it will reduce the memory cost and require a single set and prune operation
ts/index.ts
Outdated
| // after 128 messages or 1s | ||
| this.acceptFromWhitelist.set(id, { | ||
| messagesAccepted: 0, | ||
| acceptUntil: now + ACCEPT_FROM_WHITE_LIST_DURATION_MS |
| expect(scoreSpy.getCall(2)).to.be.undefined | ||
| }) | ||
|
|
||
| // TODO: run this in a unit test setup |
There was a problem hiding this comment.
there's an issue that sinon causes the test to never finish in firefox environment. I think current test set up is like an e2e test where we'll run it in different OSs and browsers, and sinon is not expected to run on it, we need a separate test:unit where we can stub any functions
Motivation
Description
decayIntervaltime periodcomputeScore()isacceptFrom()function which is called per incoming message. If peer score >= 0 we allow up to 128 messages and 1s not to compute score (this is safe enough consideringgrayListThreshold=-16000Thanks Teku for the work libp2p/jvm-libp2p#184