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

Added new plugin type: PluginScoreLedger #7617

Closed
wants to merge 2 commits into from

Conversation

wolneykien
Copy link

With a PluginScoreLedger plugin it is now possible to replace the
default BitSwap decision logic with a different one. The plugin
support depends on the experimental BitSwap option WithScoreLedger
— see 80c1a81b63e5 and related commits in the go-bitswap project
and also ebf11a6556cc commit in the go-ipfs-config project.

A map of score ledgers and new RegisterScoreLedger function were
added to core/node/core.go in order to support score ledger
injection. The selection of the particular ledger is now possible with
enchanced OnlineExchange() function accepting the name of the
ledger ("" is used for the default behavior). It is configured with
Experimental.WithScoreLedger configuration option.

In order to check this modifications and plugin loading two new tests
were added to test/integration package: TestScoreLedgerNotLoaded
and TestScoreLedgerLoadStartStop. To minimize code duplication I've
decided to slightly modify the core/mock/mock.go adding new
NewMockNodeWithConfig() function there. The code that loads and
injects plugin objects is placed in the plugins_common_test.go to
help implementing other plugin tests.

@welcome
Copy link

welcome bot commented Aug 22, 2020

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines

    if they apply.

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.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

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.
We are very grateful for your contribution!

@wolneykien
Copy link
Author

I've also issued ipfs/go-bitswap#430 — the corresponding modification to BitSwap and ipfs/go-ipfs-config#109 — the corresponding modification to the Config.

@wolneykien
Copy link
Author

wolneykien commented Aug 23, 2020

I see that 2 sharness tests have failed due to some expectations in stat0 and stat1 files. But I don't know how to see the actual content of that files.

@aschmahmann
Copy link
Contributor

aschmahmann commented Sep 3, 2020

I see that 2 sharness tests have failed due to some expectations in stat0 and stat1 files. But I don't know how to see the actual content of that files.

If you run the individual sharness tests manually on your machine (e.g. run make test_sharness_deps in the go-ipfs folder, and then `./t0125-twonode -v [note, you can add -i to terminate the test on first error]) then when the test ends there should be a trash folder where you can examine the files.

Take a look at https://github.com/ipfs/go-ipfs/tree/master/test/sharness for more info

@wolneykien
Copy link
Author

If you run the individual sharness tests manually on your machine (e.g. run make test_sharness_deps in the go-ipfs folder, and then `./t0125-twonode -v [note, you can add -i to terminate the test on first error]) then when the test ends there should be a trash folder where you can examine the files.

Thanks, @aschmahmann . The twonode test seems to be unstable: in the first run it said "failed 8 among 63", but "passed all 63 test(s)" in the second run.

With a PluginScoreLedger plugin it is now possible to replace the
default BitSwap decision logic with a different one. The plugin
support depends on the experimental BitSwap option `WithScoreLedger`
--- see 80c1a81b63e5 and related commits in the `go-bitswap` project
and also ebf11a6556cc commit in the `go-ipfs-config` project.

A map of score ledgers and new `RegisterScoreLedger` function were
added to `core/node/core.go` in order to support score ledger
injection. The selection of the particular ledger is now possible with
enchanced `OnlineExchange()` function accepting the name of the
ledger (`""` is used for the default behavior). It is configured with
`Experimental.WithScoreLedger` configuration option.

In order to check this modifications and plugin loading two new tests
were added to `test/integration` package: `TestScoreLedgerNotLoaded`
and `TestScoreLedgerLoadStartStop`. To minimize code duplication I've
decided to slightly modify the `core/mock/mock.go` adding new
`NewMockNodeWithConfig()` function there. The code that loads and
injects plugin objects is placed in the `plugins_common_test.go` to
help implementing other plugin tests.
@wolneykien
Copy link
Author

Just have synced this branch with go-ipfs/master and it has passed sharness on my node.

@aschmahmann
Copy link
Contributor

aschmahmann commented Sep 4, 2020

@wolneykien the currently supported plugins are:

  • Datastore
  • Tracer
  • IPLD
  • Daemon
  • DaemonInternal

Adding a specific plugin for one option type in Bitswap seems like overkill to me, and would open the door to adding new plugins for many many more features. The ones we have so far have been useful for many community projects.

You could accomplish what you're looking for by just building go-ipfs as a library and passing in a custom Bitswap constructor now that your Bitswap PR has landed. You may also be able to get away with a DaemonInternal plugin.

If neither of the above options are suitable for your use case, then we can investigate what makes the most sense.

@wolneykien
Copy link
Author

Adding a specific plugin for one option type in Bitswap seems like overkill to me,

May be you're right. Now, I'm not sure about a specific plugin type. However, I think it would be nice to have a way to pass that (and any other!) option to Bitswap from an ordinary plugin, say, from a Daemon plugin, isn't it?

You could accomplish what you're looking for by just building go-ipfs as a library

Thanks, that seems to be a good idea!

You may also be able to get away with a DaemonInternal plugin.

Yes. Anyway, for my current experiments I need to access the P2P Host interface that is inaccessible from CoreAPI.

@wolneykien wolneykien closed this Sep 4, 2020
@aschmahmann aschmahmann mentioned this pull request Sep 4, 2020
@aschmahmann
Copy link
Contributor

aschmahmann commented Sep 4, 2020

@wolneykien the DaemonInternal plugin gives you access to the *IpfsNode which has access to the libp2p host unlike the Daemon plugin which only gives you access to the CoreAPI.

I've also filed #7653 and #7652 which I think are relevant to the types of plugins you're asking for.

If you got the go-ipfs as a library route you might benefit from looking at #7602 which is probably the easiest way to get at Bitswap options until something like #7652 lands.

@wolneykien
Copy link
Author

#7653 and #7652 sound great! Thanks.

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