-
Notifications
You must be signed in to change notification settings - Fork 112
Conversation
This is the first step to make external decision logic (tagging peers with score values) possible. The wantlist still resides in the original `ledger` struct while sent/received byte accounting and scores are extracted to the new `scoreledger` struct managed by the original `scoreWorker()` logic. The accounting is integrated into the `Engine` via `ScoreLedger` interface making it possible to replace the original `scoreWorker()` with some other logic. The interface, however, doesn't allow a score logic to directly touch peer tags: the logic may decide about score values while tagging itself is still under control of Engine. Note: with this commit it's yet not possible to replace the original score logic because there is no public methods for that.
New `WithScoreLedger(decision.ScoreLedger)` option in the `bitswap` package is the way to connect a custom `ScoreLedger` implementation to the decision engine. The `Engine` now has the corresponding `UseScoreLedger(ScoreLedger)` method. The `ScoreLedger` and `ScorePeerFunc` types are exposed from the internal `decision` package to the public one. Because its options are processed by the `Bitswap` after construction of its parts but before starting of the engine, the default `scoreLedger` initialization is moved from `newEngine()` to `StartWorkers()`. New `TestWithScoreLedger` test is added. The test checks for start and stop of the testing score ledger implementation that is specified via `WithScoreLedger` option.
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
The related discussion is here: https://discuss.ipfs.io/t/how-to-extend-a-bitswap-strategy/7960/3 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of nits but this looks good overall
Having a separate `Init(ScoreFunc)` method seems redundant (thx @dirkmc for pointing about that). As a bonus, the two-step ledger starting process is now enclosed in the `startScoreLedger()` function.
The `Close()` method was there to stop the ledger. Let call it `Stop()` now.
Explicitly form it as the final resort.
Ping? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't see your commits until you pinged just now :)
Thanks! What about the related ipfs/kubo#7617 ? |
I'll ping the go-ipfs folks |
First, I've separated the decision engine ledger on two parts: score and the wantlist. The wantlist still resides in the original
ledger
struct while sent/received byte accounting and scores are extracted to the newscoreledger
struct managed by the originalscoreWorker()
logic. The accounting is integrated into theEngine
viaScoreLedger
interface making it possible to replace the originalscoreWorker()
with some other logic. The interface, however, doesn't allow a score logic to directly touch peer tags: the logic may decide about score values while tagging itself is still under control of Engine.Then I've added new
WithScoreLedger(decision.ScoreLedger)
option in thebitswap
package is the way to connect a customScoreLedger
implementation to the decision engine. TheEngine
now has the correspondingUseScoreLedger(ScoreLedger)
method.The
ScoreLedger
andScorePeerFunc
types are exposed from the internaldecision
package to the public one.New
TestWithScoreLedger
test is added. The test checks for start and stop of the testing score ledger implementation that is specified viaWithScoreLedger
option.