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

various fixes #490

Merged
merged 28 commits into from
Jan 5, 2015
Merged

various fixes #490

merged 28 commits into from
Jan 5, 2015

Conversation

jbenet
Copy link
Member

@jbenet jbenet commented Jan 4, 2015

  • lots of logging
  • prefix logger
  • a few bugfixes
  • disable utp

@jbenet
Copy link
Member Author

jbenet commented Jan 4, 2015

rebased on master

@jbenet jbenet force-pushed the more-dht-fixes branch 2 times, most recently from ce59ff7 to 733e281 Compare January 5, 2015 00:09
@jbenet jbenet changed the title more dht fixes various fixes Jan 5, 2015
@jbenet
Copy link
Member Author

jbenet commented Jan 5, 2015

All tests (including dockertest!) mostly* pass. Fixed #488. I'm going to merge this into master as it's way better than what's there now. Note: this does get rid of utp (see #58 (comment))


Still occasionally get these two failures (merging this PR, but we should fix them elsewhere):

  • fuse/ipns/TestFastRepublish
--- FAIL: TestFastRepublish (0.19 seconds)
    ipns_test.go:255: resolve err: QmaZ99PKvTChSDUy7LZ98A2CyWK3hWovX3ZaqN66yPRoTz datastore: key not found
FAIL
FAIL    github.com/jbenet/go-ipfs/fuse/ipns 1.867s

i haven't looked into this. i recently changed the core/mock routing to use routing/mock, which fixed another ipns test. if routing/mock is working, this is an actual ipns bug. @whyrusleeping @briantigerchow thoughts?

  • routing/dht/TestGetFailures
--- FAIL: TestGetFailures (20.29 seconds)
    ext_test.go:59: Timeout test passed.
    ext_test.go:90: Expected ErrNotFound, got: context deadline exceeded
FAIL

dht is not returning ErrNotFound properly, but instead getting stuck until context (which is pretty long now) expires. the expected behavior is correct. this needs to be fixed.

@whyrusleeping could you take a look at these?

@jbenet
Copy link
Member Author

jbenet commented Jan 5, 2015

Actually, before i merge, looks like jenkins is almost back to passing: https://build.protocol-dev.com/job/all-the-tests/373/

Remaining issue: https://build.protocol-dev.com/job/sharnessss/385/console -- @briantigerchow thoughts?

@jbenet
Copy link
Member Author

jbenet commented Jan 5, 2015

rebased on top of master again, to get changes in #492 and maybe diagnose sharness problem.

jbenet and others added 16 commits January 5, 2015 00:16
this commit adds a logger with prefixes
we shouldn't use an arbitrary timeout here. since Get
doesnt take in a context yet, we give a large upper bound.
think of an http request. we want it to go on as long as
the client requests it.
utp is BROKEN!! it causes tests to fail.
@jbenet @whyrusleeping

This bug (missing return) could tie up the client worker and cause
operations to come to a halt.
Without `-f`, `make clean` fails on machines that don't have the dir.

cc @jbenet
this fixes a failing ipns test which didnt have
a "working" routing system
TestGetFailures may just be operating very slowly, instead
of completely failing. Right now it gets caught on travis
often. not sure if its actually wrong.
mocknet indeterminism screwed this test up. that's twice
it's bitten us. let's not let it do it a third time.

cc @briantigerchow omg.
This is causing test failures because tests don't usually
have "/-/-" format. we can decide whether or not to allow
keys without validators, but for now removing.

cc @whyrusleeping
We use make test as the measure of correctness.
This laxity has let bugs creep into several systems.
This commit changes our target to always run expensive
tests, unless one specifically runs `make test_short`

(we would do well to remove most if not all timing--
that's usually what makes tests take a long time.)
Bitswap doesn't usually care about dialing. the underlying
network adapter can make sure of that.
@jbenet
Copy link
Member Author

jbenet commented Jan 5, 2015

jbenet added a commit that referenced this pull request Jan 5, 2015
@jbenet jbenet merged commit c065bcc into master Jan 5, 2015
@jbenet jbenet deleted the more-dht-fixes branch January 5, 2015 13:59
@jbenet jbenet removed the status/in-progress In progress label Jan 5, 2015
@jbenet jbenet mentioned this pull request Jan 5, 2015
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.

1 participant