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

Directly wire ctx into http request #1997

Closed
wants to merge 74 commits into from
Closed

Conversation

rht
Copy link
Contributor

@rht rht commented Nov 23, 2015

closes #1993

daviddias and others added 30 commits November 11, 2015 10:04
License: MIT
Signed-off-by: David Dias <daviddias.p@gmail.com>
This used to lead to large refcount numbers, causing Flush to create a
lot of IPFS objects, and merkledag to consume tens of gigabytes of
RAM.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
OS X sed is documented as "-i SUFFIX", GNU sed as "-iSUFFIX". The one
consistent case seems to be "-iSUFFIX", where suffix cannot empty (or
OS X will parse the next argument as the suffix).

This used to leave around files named `refsout=` on Linux, and was
just confusing.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
These secondary copies were never actually queried, and didn't contain
the indirect refcounts so they couldn't become the authoritative
source anyway as is. New goal is to move pinning into IPFS objects.

A migration will be needed to remove the old data from the datastore.
This can happen at any time after this commit.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
Pinner had method GetManual that returned a ManualPinner, so every
Pinner had to implement ManualPinner anyway.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
Platform-dependent behavior is not nice, and negative refcounts are
not very useful.

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

sharness: Don't assume we know all things that can create garbage

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
WARNING: No migration performed! That needs to come in a separate
commit, perhaps amended into this one.

This is the minimal rewrite, only changing the storage from
JSON(+extra keys) in Datastore to IPFS objects. All of the pinning
state is still loaded in memory, and written from scratch on Flush. To
do more would require API changes, e.g. adding error returns.

Set/Multiset is not cleanly separated into a library, yet, as it's API
is expected to change radically.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
…lure

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
There was doublewrapping with an unneeded msgio. given that we
use a stream muxer now, msgio is only needed by secureConn -- to
signal the boundaries of an encrypted / mac-ed ciphertext.

Side note: i think including the varint length in the clear is
actually a bad idea that can be exploited by an attacker. it should
be encrypted, too. (TODO)

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
* 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>
The addition of a locking interface to the blockstore allows us to
perform atomic operations on the underlying datastore without having to
worry about different operations happening in the background, such as
garbage collection.

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
This commit improves (fixes) the FetchGraph call for recursively
fetching every descendant node of a given merkledag node. This operation
should be the simplest way of ensuring that you have replicated a dag
locally.

This commit also implements a method in the merkledag package called
EnumerateChildren, this method is used to get a set of the keys of every
descendant node of the given node. All keys found are noted in the
passed in KeySet, which may in the future be implemented on disk to
avoid excessive memory consumption.

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

dont GC blocks used by pinner

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

comment GC algo

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

add lock to blockstore to prevent GC from eating wanted blocks

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

improve FetchGraph

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

separate interfaces for blockstore and GCBlockstore

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

reintroduce indirect pinning, add enumerateChildren dag method

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping and others added 9 commits November 17, 2015 10:17
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
if bucket doesnt have enough peers, grab more elsewhere
add closenotify and large timeout to gateway
License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
Add config option for flatfs no-sync
License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@jbenet jbenet added the status/in-progress In progress label Nov 23, 2015
@whyrusleeping
Copy link
Member

drop the goroutine that the 'Do' is in entirely, and remove the select channel stuff, see 590e1c8

@whyrusleeping
Copy link
Member

@rht i'm out for the day, but if you can figure out how to get CloseNotify working again, I would be soooooo happy

License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
}

func NewClient(address string) Client {
// We cannot use the default transport because of a bug in go's connection reuse
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to check against this case?

Copy link
Member

Choose a reason for hiding this comment

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

this comment can be removed. We're good now that we've switched to go1.5

License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@rht rht force-pushed the wire/ctx-http-request-2 branch from 3109b59 to 16e93ab Compare November 23, 2015 15:06
@@ -39,6 +39,7 @@ var log = logging.Logger("cmd/ipfs")
var (
errUnexpectedApiOutput = errors.New("api returned unexpected output")
errApiVersionMismatch = errors.New("api version mismatch")
errRequestCanceled = errors.New("request canceled")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

err defined. I will make it exportable once I've found the other place that needs the ctx-cancel-wrapped.

@rht rht added the RFCR label Nov 24, 2015
return &client{
serverAddress: address,
httpClient: http.Client{
Transport: &http.Transport{
DisableKeepAlives: true,
Copy link
Member

Choose a reason for hiding this comment

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

do we still want to disable keep alives or no?

@jbenet
Copy link
Member

jbenet commented Nov 30, 2015

this LGTM in general. but want a test to make sure this behaves as we expect it to-- i've been misled by interpretations of standard libs before.

@rht
Copy link
Contributor Author

rht commented Dec 16, 2015

Though in this case, using the stdlib prevents a class of race conditions.
Test is possible once #2032 is merged.

@whyrusleeping
Copy link
Member

@rht we've got CloseNotify fixed now, mind rebasing this one on master?

@rht rht closed this Jan 30, 2016
@rht
Copy link
Contributor Author

rht commented Jan 30, 2016

This could have been merged with/without the CloseNotify.

@Kubuxu Kubuxu deleted the wire/ctx-http-request-2 branch February 27, 2017 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants