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

fix: re-sort queue after adding tasks #221

Merged
merged 1 commit into from
May 7, 2020
Merged

fix: re-sort queue after adding tasks #221

merged 1 commit into from
May 7, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented May 7, 2020

When tasks are added to an existing list of tasks for a given peer, we need to sort the queue to ensure the order is correct, otherwise we never process pending tasks as task lists with pending tasks need to be moved up the queue.

Fixes the build problems exposed in ipfs/js-ipfs#2992

Also upgrades aegir to a safe version.

When tasks are added to an existing list of tasks for a given peer,
we need to sort the queue to ensure the order is correct, otherwise
we never process pending tasks as task lists with pending tasks need
to be moved up the queue.

Fixes the build problems exposed in ipfs/js-ipfs#2992

Also upgrade aegir to a safe version.
achingbrain added a commit that referenced this pull request May 7, 2020
Builds on the work in #221

I see intermittant but frequent CI errors with js-ipfs, usually after
a test has finished and the ndoes are being torn down.  Getting it
to do some additional logging reveals bitswap is crashing when it
cannot send a message to a remote peer due to the libp2p dial failing:

```console
ipfs: [stdout] Error: stream ended before 1 bytes became available
ipfs:     at /home/travis/build/ipfs/js-ipfs/node_modules/it-reader/index.js:37:9
ipfs:     at processTicksAndRejections (internal/process/task_queues.js:97:5)
ipfs:     at async /home/travis/build/ipfs/js-ipfs/node_modules/it-length-prefixed/src/decode.js:80:20
ipfs:     at async oneChunk (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:12:20)
ipfs:     at async Object.exports.read (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:34:15)
ipfs:     at async module.exports (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/select.js:21:19)
ipfs:     at async ClassIsWrapper.newStream [as _newStream] (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p/src/upgrader.js:251:40)
ipfs:     at async ClassIsWrapper.newStream (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p-interfaces/src/connection/connection.js:172:34)
ipfs:     at async Network.sendMessage (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/network.js:147:34)
ipfs:     at async DecisionEngine._processTasks (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/decision-engine/index.js:124:5) {
ipfs:   code: 'ERR_UNSUPPORTED_PROTOCOL',
ipfs:   buffer: BufferList { _bufs: [], length: 0 }
ipfs: }
```

This PR adds a try/catch around network send operations.  At the moment
it just dumps the request, I'm not sure if we want to add a retry in
there or something.
Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

Nice catch 👍

@dirkmc dirkmc merged commit 1a5ed4a into master May 7, 2020
dirkmc pushed a commit that referenced this pull request May 7, 2020
* fix: survive not being able to send messages to peers

Builds on the work in #221

I see intermittant but frequent CI errors with js-ipfs, usually after
a test has finished and the ndoes are being torn down.  Getting it
to do some additional logging reveals bitswap is crashing when it
cannot send a message to a remote peer due to the libp2p dial failing:

```console
ipfs: [stdout] Error: stream ended before 1 bytes became available
ipfs:     at /home/travis/build/ipfs/js-ipfs/node_modules/it-reader/index.js:37:9
ipfs:     at processTicksAndRejections (internal/process/task_queues.js:97:5)
ipfs:     at async /home/travis/build/ipfs/js-ipfs/node_modules/it-length-prefixed/src/decode.js:80:20
ipfs:     at async oneChunk (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:12:20)
ipfs:     at async Object.exports.read (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:34:15)
ipfs:     at async module.exports (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/select.js:21:19)
ipfs:     at async ClassIsWrapper.newStream [as _newStream] (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p/src/upgrader.js:251:40)
ipfs:     at async ClassIsWrapper.newStream (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p-interfaces/src/connection/connection.js:172:34)
ipfs:     at async Network.sendMessage (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/network.js:147:34)
ipfs:     at async DecisionEngine._processTasks (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/decision-engine/index.js:124:5) {
ipfs:   code: 'ERR_UNSUPPORTED_PROTOCOL',
ipfs:   buffer: BufferList { _bufs: [], length: 0 }
ipfs: }
```

This PR adds a try/catch around network send operations.  At the moment
it just dumps the request, I'm not sure if we want to add a retry in
there or something.
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