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

Serial File Opening #553

Merged
merged 5 commits into from
Jan 14, 2015
Merged

Serial File Opening #553

merged 5 commits into from
Jan 14, 2015

Conversation

mappum
Copy link
Contributor

@mappum mappum commented Jan 13, 2015

This resolves #536.

It creates a new kind of commands.File called SerialFile, which only needs to open one os.File at a time. It's hella clean since it handles all of the file opening/closing on its own.

@btc btc added the status/in-progress In progress label Jan 13, 2015
return f.Reader.Read(p)
}

func (f *ReaderFile) Close() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to accept a ReadCloser and allow clients to wrap their non-closing Readers in a Closer wrapper (one which has no effect).

Secretly introspecting and closing a client's Reader is harder to explain to callers and can result in surprising behavior.

Copy link
Member

Choose a reason for hiding this comment

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

yeah i got bitten by this :/ -- i've since stopped doing it. it's also fine to leave closing to the user in many cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

just for the record the NopCloser is here:

package ioutil

func NopCloser(r io.Reader) io.ReadCloser

http://golang.org/pkg/io/ioutil/#NopCloser

jbenet added a commit that referenced this pull request Jan 14, 2015
@jbenet jbenet merged commit 920ddc7 into master Jan 14, 2015
@jbenet jbenet removed the status/in-progress In progress label Jan 14, 2015
@jbenet jbenet deleted the serial-file branch January 14, 2015 07:30
@hmeine
Copy link

hmeine commented Sep 22, 2015

Strange, I am just starting to play around with ipfs, and apparently got exactly this error:

ERRO[20:25:47:000] open /Users/hmeine/.ipfs/blocks/1220c6bc/put-911313602: too many open files  module=commands/http

This was with ipfs add -r <some_dir>, after roughly a dozen files (and at least two subdirectories being discovered).
I installed ipfs today, using go get -u github.com/ipfs/go-ipfs/cmd/ipfs, and ipfs version states 0.3.8-dev, so I expect the above changes to be in my version?
Should I (as a beginner) rather install a release version? Having go installed (disclaimer: I have no experience with it, though), and being familiar with Git and OSS development, I preferred the latest version. Is that considered much more bleeding edge / unstable than the bundled downloads?

@whyrusleeping
Copy link
Member

@hmeine continue discussion on #1689

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.

ipfs add opening too many files
5 participants