Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

state machine, transitions and events #1061

Closed
pgte opened this issue Nov 6, 2017 · 1 comment
Closed

state machine, transitions and events #1061

pgte opened this issue Nov 6, 2017 · 1 comment
Assignees
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@pgte
Copy link
Contributor

pgte commented Nov 6, 2017

  • Version: 0.26.0
  • Platform: All
  • Subsystem: core, repo, init, swarm, bitswap, libp2p, perhaps others..

Type:

Enhancement

Severity:

Medium

Description:

Initialisation and stopping an IPFS node envolves many states and transitions that are hard to handle, can fail and have many corner cases.

Currently, initialisation states and their transitions are verified and managed explicitly at code level. From my experience, this almost invariantly conducts to bugs and weirdness in app start code.

Instead, a finite state machine that contains all the main, intermediary and error states should be well defined and documented.

The initialisation code should be driven by state transitions, and not the other way around (like it is now). This will avoid double user actions, like starting or stopping the node twice concurrently, or trying to stop after stopped, etc.


IMO, special attention should be given so that the implementation:

  • is consistent: for instance, even if repo initialisation is not required, a initialised or equivalent transition event should be fired.
  • Some operations should be idempotent: starting a node that is started or is starting should not lead to an error, much less triggering the startup procedure again.
  • Any Irrecoverable error in initialisation should lead to an dead-end "error" state.
@daviddias
Copy link
Member

@pgte you have my full blessing to push the changes you have been describing :)

@daviddias daviddias added status/ready Ready to be worked exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue P1 High: Likely tackled by core team if no one steps up labels Jan 25, 2018
alanshaw pushed a commit that referenced this issue Oct 17, 2019
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options.

It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add).

In order to boot up a new ipfsx node I refactored the boot procedure to enable the following:

1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests.
1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438
1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257
1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future.

**FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion?

resolves #1438
resolves #1061
resolves #2257
refs #2509
refs #1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
alanshaw pushed a commit that referenced this issue Dec 9, 2019
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options.

It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add).

In order to boot up a new ipfsx node I refactored the boot procedure to enable the following:

1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests.
1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438
1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257
1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future.

**FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion?

resolves #1438
resolves #1061
resolves #2257
refs #2509
refs #1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
alanshaw pushed a commit that referenced this issue Dec 10, 2019
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options.

It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add).

In order to boot up a new ipfsx node I refactored the boot procedure to enable the following:

1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests.
1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438
1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257
1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future.

**FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion?

resolves #1438
resolves #1061
resolves #2257
refs #2509
refs #1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
alanshaw pushed a commit that referenced this issue Jan 23, 2020
A roundup of the following PRs:

* closes #2658
* closes #2660
* closes #2661
* closes #2668
* closes #2674
* closes #2676
* closes #2680
* test fixes and other fix ups

---

To allow us to pass the interface tests, the timeout option is now supported in the `object.get` and `refs` APIs. It doesn't actually cancel the operation all the way down the stack, but allows the method call to return when the timeout is reached.

https://github.com/ipfs/js-ipfs/pull/2683/files#diff-47300e7ecd8989b6246221de88fc9a3cR170

---

Supersedes #2724

---

resolves #1438
resolves #1061
resolves #2257
resolves #2509
resolves #1670
refs ipfs-inactive/interface-js-ipfs-core#394

BREAKING CHANGE: `IPFS.createNode` removed

BREAKING CHANGE: IPFS is not a class that can be instantiated - use `IPFS.create`. An IPFS node instance is not an event emitter.

BREAKING CHANGE: Instance `.ready` property removed. Please use `IPFS.create` instead.

BREAKING CHANGE: Callbacks are no longer supported on any API methods. Please use a utility such as [`callbackify`](https://www.npmjs.com/package/callbackify) on API methods that return Promises to emulate previous behaviour.

BREAKING CHANGE: `PeerId` and `PeerInfo` classes are no longer statically exported from `ipfs-http-client` since they are no longer used internally.

BREAKING CHANGE: `pin.add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.

BREAKING CHANGE: `pin.ls` now returns an async iterable.

BREAKING CHANGE: `pin.ls` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.

BREAKING CHANGE: `pin.rm` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.

BREAKING CHANGE: `add` now returns an async iterable.

BREAKING CHANGE: `add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.

BREAKING CHANGE: `addReadableStream`, `addPullStream` have been removed.

BREAKING CHANGE: `ls` now returns an async iterable.

BREAKING CHANGE: `ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.

BREAKING CHANGE: `files.readPullStream` and `files.readReadableStream` have been removed.

BREAKING CHANGE: `files.read` now returns an async iterable.

BREAKING CHANGE: `files.lsPullStream` and `files.lsReadableStream` have been removed.

BREAKING CHANGE: `files.ls` now returns an async iterable.

BREAKING CHANGE: `files.ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.

BREAKING CHANGE: `files.ls` no longer takes a `long` option (in core) - you will receive all data by default.

BREAKING CHANGE: `files.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.

BREAKING CHANGE: `get` now returns an async iterable. The `content` property value for objects yielded from the iterator is now an async iterable that yields [`BufferList`](https://github.com/rvagg/bl) objects.

BREAKING CHANGE: `stats.bw` now returns an async iterable.

BREAKING CHANGE: `addFromStream` has been removed. Use `add` instead.

BREAKING CHANGE: `addFromFs` has been removed. Please use the exported `globSource` utility and pass the result to `add`. See the [glob source documentation](https://github.com/ipfs/js-ipfs-http-client#glob-source) for more details and an example.

BREAKING CHANGE: `addFromURL` has been removed. Please use the exported `urlSource` utility and pass the result to `add`. See the [URL source documentation](https://github.com/ipfs/js-ipfs-http-client#url-source) for more details and an example.

BREAKING CHANGE: `name.resolve` now returns an async iterable. It yields increasingly more accurate resolved values as they are discovered until the best value is selected from the quorum of 16. The "best" resolved value is the last item yielded from the iterator. If you are interested only in this best value you could use `it-last` to extract it like so:

```js
const last = require('it-last')
await last(ipfs.name.resolve('/ipns/QmHash'))
```

BREAKING CHANGE: `block.rm` now returns an async iterable.

BREAKING CHANGE: `block.rm` now yields objects of `{ cid: CID, error: Error }`.

BREAKING CHANGE: `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now all return an async iterable.

BREAKING CHANGE: `dht.findPeer`, `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now yield/return an object `{ id: CID, addrs: Multiaddr[] }` instead of a `PeerInfo` instance(s).

BREAKING CHANGE: `refs` and `refs.local` now return an async iterable.

BREAKING CHANGE: `object.data` now returns an async iterable that yields `Buffer` objects.

BREAKING CHANGE: `ping` now returns an async iterable.

BREAKING CHANGE: `repo.gc` now returns an async iterable.

BREAKING CHANGE: `swarm.peers` now returns an array of objects with a `peer` property that is a `CID`, instead of a `PeerId` instance.

BREAKING CHANGE: `swarm.addrs` now returns an array of objects `{ id: CID, addrs: Multiaddr[] }` instead of `PeerInfo` instances.

BREAKING CHANGE: `block.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `key` property.

BREAKING CHANGE: `bitswap.wantlist` now returns an array of [CID](https://github.com/multiformats/js-cid) instances.

BREAKING CHANGE: `bitswap.stat` result has changed - `wantlist` and `peers` values are now an array of [CID](https://github.com/multiformats/js-cid) instances.

BREAKING CHANGE: the `init` option passed to the IPFS constructor will now not take any initialization steps if it is set to `false`. Previously, the repo would be initialized if it already existed. This is no longer the case. If you wish to initialize a node but only if the repo exists, pass `init: { allowNew: false }` to the constructor.

BREAKING CHANGE: removed `file ls` command from the CLI and HTTP API.

BREAKING CHANGE: Delegated peer and content routing modules are no longer included as part of core (but are still available if starting a js-ipfs daemon from the command line). If you wish to use delegated routing and are creating your node _programmatically_ in Node.js or the browser you must `npm install libp2p-delegated-content-routing` and/or `npm install libp2p-delegated-peer-routing` and provide configured instances of them in [`options.libp2p`](https://github.com/ipfs/js-ipfs#optionslibp2p). See the module repos for further instructions:

- https://github.com/libp2p/js-libp2p-delegated-content-routing
- https://github.com/libp2p/js-libp2p-delegated-peer-routing
MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this issue May 22, 2020
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants