feature: ability to override default bitswap max message size #152

ya7ya · 2017-09-27T18:16:09Z

This PR is a part of the fix for the browser to browser file transfer issue in IPFS ipfs/js-ipfs#1019 .

This edit allows an override of MAX_MESSAGE_SIZE so it can be used in the browser via WebRTC Transport. I been testing and so far 32KB seem to work in the browser without getting throttled.

License: MIT
Signed-off-by: Yahya ya7yaz@gmail.com

License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

codecov · 2017-09-27T18:23:18Z

Codecov Report

Merging #152 into master will increase coverage by 0.96%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #152      +/-   ##
==========================================
+ Coverage   91.16%   92.13%   +0.96%     
==========================================
  Files          14       14              
  Lines         736      661      -75     
==========================================
- Hits          671      609      -62     
+ Misses         65       52      -13

Impacted Files	Coverage Δ
src/decision-engine/index.js	`94.96% <100%> (+6.59%)`	⬆️
src/index.js	`82.14% <100%> (+0.12%)`	⬆️
src/want-manager/index.js	`88.13% <0%> (-6.78%)`	⬇️
src/types/wantlist/index.js	`92.85% <0%> (-3.58%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6175f9c...f9d21c4. Read the comment docs.

ya7ya · 2017-10-09T13:39:06Z

hey @diasdavid , Can you take a look at this PR?
This minor modification allows bitswap to change MAX_MESSAGE_SIZE so it can be used in the browser via WebRTC Transport. I been testing and so far 32KB seem to work in the browser without getting throttled.

Let me know what you think.

daviddias

Hi @ya7ya, this overall LGTM, just needs 2 more things:

Interop tests to be added at the js-ipfs level. By dropping the max message size, we need to ensure that we still accept go-ipfs sent messages (and other nodes that didn't change this factor)
Documentation update (through jsdoc)

Stebalien · 2017-10-10T22:01:42Z

@diasdavid shouldn't this be handled by the WebRTC transport?

…e nodes License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

* added missing multicodec dependency * using ~ for multicodec dependency range because version < 1

* test: close IPFSRepo before deleting its files * chore: run on appveyor * chore: upgrade packages * chore: debugging appveyor failure * test: more appveyor debugging * test: use os.tmpdir() not '/tmp' * test: timeout issue * test: fix error reporting * test: Math.Random => uuid Math.Random can create collisions. Using uuid should not create a collision. * test: all tests can timeout, need global timeout specified * test: bitswap-mock multi peer is just too long, skip it * fix package.json * test: ipfs@0.18.3 breaks us. reverting to 0.18.2 * chore: update repo version * chore: update deps * bring back the test

This commit updates all CI scripts to the latest version

* bitswap stats: async updater * linter happy * pre-commit hook for linting and tests * stats: support dataReceived * stats: data received * stats: blocks sent and data sent * stats: using bignum * stats: compute throttle with threshold * stats: update timeout is now dynamic based on queue size * stats: moving averages * stats: providesBufferLength * stats: support for providesBufferLength and wantListLength * stats: support for peerCount * stats: enable / disable * increased test timeout

* getMany: guaranteed ordering * tests: swarm, humble beginnings * chore: added test for when no cid is passed in * chore: more swarm tests * tests: serializing interconnections on swarm tests * more swarm tests enabled * tests: removed some onlys * tests: serializing interconnections on swarm tests * tests: added double get test * chore: added test for double get * one less callback function * chore: update libp2p version to latest * chore: update remaining deps

daviddias · 2018-01-28T02:53:36Z

@ya7ya is this PR something you still think you can finish?

* fix: getMany: ensuring we set the want list independently of having all blocks or not * bitswap stats tests: not requiring specific order to finish * increasing the timeouts of swarm tests

ya7ya · 2018-01-28T18:55:08Z

@diasdavid Hey david, I completely forgot about this PR, I'm so sorry about this. I'll try to finalize this and add the required tests as soon as possible. 🙇

* docs: current stats API documented * per peer stats

License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

…e nodes License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

…into feat/message-size License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

Stebalien · 2018-02-06T16:34:49Z

There are a lot of changes here so it's a bit hard to tell what this is actually doing now but, if this is doing what the title says, this is very much the wrong fix. Is it really not possible (and much simpler) to have the WebRTC transport write at most 32KiB at a time?

ya7ya · 2018-02-06T16:38:55Z

hey @Stebalien , I rebased this PR to master so that's why there are alotta changes.

if this is doing what the title says, this is very much the wrong fix.

Yes. This does what the title says. And I'd be very interested in doing the correct fix and rejecting this PR if it's possible to override this at webRTC transport level.

Stebalien · 2018-02-06T18:08:49Z

It should be fairly easy to fix in the webRTC transport. Unfortunately, my understanding of js streams is rather limited (I generally avoid working on javascript).

ya7ya · 2018-02-06T23:11:14Z

@Stebalien I'm gonna look into it. probably we can throttle the pull request conn in the Transport itself. I'm not 100% sure but I'll see what I can do. Thanks for the tip, I appreciate your input 👍 💯

dryajov · 2018-02-07T02:19:39Z

@ya7ya I believe, we can use something like this to chunk the stream at the transport level - https://pull-stream.github.io/#pull-block.

alanshaw · 2019-12-12T09:42:31Z

+1 this needs to be handled in the webrtc libp2p transport.

daviddias · 2019-12-12T09:51:24Z

@vasco-santos, @jacobheun this PR suggests that the transports should get some benchmarking treatment.

There are two low hanging fruits to make things fast:

The change that @ya7ya suggests for the WebRTC transport
Creating a simple-peer module that doesn't rely on Readable Streams

achingbrain · 2020-05-27T15:27:14Z

I don't think this PR is necessary any more, the upstream issue it's supposed to fix has been addressed by libp2p refactors.

Please feel free to reopen if it's still a problem.

feature: ability to override default bitswap max message size

d1a13f2

License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

ya7ya requested a review from dignifiedquire September 27, 2017 18:16

ya7ya requested a review from daviddias September 28, 2017 12:50

daviddias requested changes Oct 9, 2017

View reviewed changes

setting a minimum msg size, tests for go-interop and variable msg siz…

bd50ee3

…e nodes License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

daviddias added status/ready Ready to be worked P1 High: Likely tackled by core team if no one steps up labels Oct 13, 2017

chore: update deps

339503d

ya7ya mentioned this pull request Oct 25, 2017

Select custom Chunk Sizes ipfs/js-ipfs#1019

Closed

pgte and others added 13 commits November 8, 2017 16:03

fix: add missing multicodec dependency (ipfs#155)

751d436

* added missing multicodec dependency * using ~ for multicodec dependency range because version < 1

chore: update deps

fa797a0

chore: update contributors

1a62f49

chore: release version v0.17.3

ab69719

chore: update contributors

1eea363

chore: release version v0.17.4

91aaedc

chore: update deps

f2c8ea2

chore: updating CI files (ipfs#157)

6886a59

This commit updates all CI scripts to the latest version

chore: update contributors

095f467

chore: release version v0.18.0

349b3bc

daviddias approved these changes Jan 28, 2018

View reviewed changes

fix: getMany: ensuring we set the want list (ipfs#162)

8e91def

* fix: getMany: ensuring we set the want list independently of having all blocks or not * bitswap stats tests: not requiring specific order to finish * increasing the timeouts of swarm tests

feat: per-peer stats (ipfs#166)

ff978d0

* docs: current stats API documented * per peer stats

pgte and others added 7 commits February 6, 2018 10:22

feat: added getMany performance tests (ipfs#164)

b349085

chore: update deps

89b263a

chore: update contributors

1fc73b4

chore: release version v0.18.1

f090590

feature: ability to override default bitswap max message size

37f19cf

License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

setting a minimum msg size, tests for go-interop and variable msg siz…

114c4cf

…e nodes License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

Merge branch 'feat/message-size' of github.com:ya7ya/js-ipfs-bitswap …

f9d21c4

…into feat/message-size License: MIT Signed-off-by: Yahya <ya7yaz@gmail.com>

AquiGorka mentioned this pull request Feb 7, 2018

Keep an eye out for an update to js-ipfs FormulaMonks/thicket#254

Open

AquiGorka mentioned this pull request Apr 12, 2018

Sharing "big" files in Chrome and Safari/Firefox over webrtc not working ipfs/js-ipfs#1307

Closed

ya7ya mentioned this pull request May 29, 2018

READY | alleviating browser throttling on WebRTC transport libp2p/js-libp2p-webrtc-star#145

Merged

ya7ya mentioned this pull request Jul 1, 2018

Stable transports in the browser ipfs/js-ipfs#1088

Closed

momack2 added this to Ready in ipfs/js-ipfs May 10, 2019

momack2 added this to Ready in ipfs/js-waffle May 10, 2019

achingbrain closed this May 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: ability to override default bitswap max message size #152

feature: ability to override default bitswap max message size #152

ya7ya commented Sep 27, 2017

codecov bot commented Sep 27, 2017 •

edited

Loading

ya7ya commented Oct 9, 2017

daviddias left a comment

Stebalien commented Oct 10, 2017

daviddias commented Jan 28, 2018

ya7ya commented Jan 28, 2018

Stebalien commented Feb 6, 2018

ya7ya commented Feb 6, 2018

Stebalien commented Feb 6, 2018

ya7ya commented Feb 6, 2018

dryajov commented Feb 7, 2018

alanshaw commented Dec 12, 2019

daviddias commented Dec 12, 2019

achingbrain commented May 27, 2020

feature: ability to override default bitswap max message size #152

feature: ability to override default bitswap max message size #152

Conversation

ya7ya commented Sep 27, 2017

codecov bot commented Sep 27, 2017 • edited Loading

Codecov Report

ya7ya commented Oct 9, 2017

daviddias left a comment

Choose a reason for hiding this comment

Stebalien commented Oct 10, 2017

daviddias commented Jan 28, 2018

ya7ya commented Jan 28, 2018

Stebalien commented Feb 6, 2018

ya7ya commented Feb 6, 2018

Stebalien commented Feb 6, 2018

ya7ya commented Feb 6, 2018

dryajov commented Feb 7, 2018

alanshaw commented Dec 12, 2019

daviddias commented Dec 12, 2019

achingbrain commented May 27, 2020

codecov bot commented Sep 27, 2017 •

edited

Loading