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.

Sign up for GitHub

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

Jump to bottom

WIP: using multistream muxer #1438

Merged
merged 1 commit into from
Jul 7, 2015
Merged

WIP: using multistream muxer #1438

merged 1 commit into from
Jul 7, 2015

Conversation

whyrusleeping
Copy link
Member

This PR adds multistream as the protocol muxer in go-ipfs and allows the selection of yamux or spdystream for stream muxing. It should be merged on top of the dev0.4.0 branch.

Currently, some tests are failing, i've looked at them briefly, but theyre out of my domain of knowledge. if @jbenet could take a look, that would be great.

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

@whyrusleeping whyrusleeping added the status/in-progress In progress label Jul 1, 2015
This was referenced Jul 1, 2015
Merged
Closed
@whyrusleeping whyrusleeping added codereview and removed status/in-progress In progress labels Jul 2, 2015
@jbenet
Copy link
Member

jbenet commented Jul 3, 2015

@whyrusleeping code LGTM but the tests seem to be deadlocking. maybe things not getting added to the muxer right? or negotiated by the muxer right?

@whyrusleeping
Copy link
Member Author

where are you seeing a deadlock? I see some tests failing locally, but nothing seems like its actually hung

@jbenet
Copy link
Member

jbenet commented Jul 3, 2015

the travis tests hang

@whyrusleeping
Copy link
Member Author

(also, If you could look at the test failures in p2p/protocol/identify that would be great)

@whyrusleeping
Copy link
Member Author

These test failures are rather frustrating... nothing appears to be concretely related... although every travis test failed on that set.

@jbenet
Copy link
Member

jbenet commented Jul 6, 2015

@whyrusleeping all circlecis failed on this:

not ok 7 - transport should be unencrypted
#   
#     go-sleep 0.5s | nc localhost "$PORT_SWARM" >swarmnc &&
#     test_must_fail grep -q "AES-256,AES-128" swarmnc &&
#     grep -q "/ipfs/identify" swarmnc ||
#     test_fsh cat swarmnc
#

so that is a bug (either on the code or the test should change to reflect new state)

@whyrusleeping
Copy link
Member Author

ah, didnt catch that one. That test fails no matter what on my machine anyways so i dont run it locally, speaking of which, i wrote this to do it better: https://github.com/whyrusleeping/ngrep

@whyrusleeping
Copy link
Member Author

look at all that green.

jbenet
jbenet reviewed Jul 7, 2015
View reviewed changes
pin/pin.go
@@ -243,7 +244,9 @@ func LoadPinner(d ds.ThreadSafeDatastore, dserv mdag.DAGService) (Pinner, error)

rootKey := key.Key(rootKeyBytes)

ctx := context.TODO()
ctx, cancel := context.WithTimeout(context.TODO(), time.Second*5)
defer cancel()
Copy link
Member

Choose a reason for hiding this comment

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

so this causes exiting with an error if loading the pinner fails, correct?

@whyrusleeping @jbenet
using multistream muxer
ef34768 
* ID service stream
* make the relay service use msmux
* fix nc tests

Note from jbenet: Maybe we should remove the old protocol/muxer
and see what breaks. It shouldn't be used by anything now.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
Signed-off-by: Juan Batiz-Benet <juan@benet.ai>
@GitCop
Copy link

GitCop commented Jul 7, 2015

There were the following issues with your Pull Request

  • Commit: ef34768
    • Invalid signoff. Commit message must end with License: MIT
      Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue

This message was auto-generated by https://gitcop.com

@jbenet
Copy link
Member

jbenet commented Jul 7, 2015

squashed altogether for tests. (previous head: 2474247)

jbenet added a commit that referenced this pull request Jul 7, 2015
@jbenet
Merge pull request #1438 from ipfs/feat/mss-mux
16ea653 
WIP: using multistream muxer
@jbenet jbenet merged commit 16ea653 into dev0.4.0 Jul 7, 2015
@jbenet jbenet deleted the feat/mss-mux branch July 7, 2015 02:05
@jbenet jbenet removed the codereview label Jul 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@whyrusleeping @jbenet @GitCop