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

Filestore possiblly not hooked up right #5121

Closed
kevina opened this issue Jun 15, 2018 · 4 comments
Closed

Filestore possiblly not hooked up right #5121

kevina opened this issue Jun 15, 2018 · 4 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@kevina
Copy link
Contributor

kevina commented Jun 15, 2018

When the filestore is used the CachedBlockstore is not according to this code (in builder.go):

	bs := bstore.NewBlockstore(rds)
	...

	cbs, err := bstore.CachedBlockstore(ctx, bs, opts)
	...

	n.BaseBlocks = cbs
	n.GCLocker = bstore.NewGCLocker()
	n.Blockstore = bstore.NewGCBlockstore(cbs, n.GCLocker)

	if conf.Experimental.FilestoreEnabled {
		...
		n.Filestore = filestore.NewFilestore(bs, n.Repo.FileManager())
		n.Blockstore = bstore.NewGCBlockstore(n.Filestore, n.GCLocker)
		...
	}

It looks like it was always this way based on 2884c84

@whyrusleeping Was this intentional or an oversight.

@kevina kevina added the kind/bug A bug in existing code (including security flaws) label Jun 15, 2018
@whyrusleeping
Copy link
Member

Oh, that looks like an oversight on my part.

@kevina
Copy link
Contributor Author

kevina commented Jun 15, 2018

@whyrusleeping So is the fix to change

n.Filestore = filestore.NewFilestore(bs, n.Repo.FileManager())

to

n.Filestore = filestore.NewFilestore(cbs, n.Repo.FileManager())

that is s/bs/cbs/?

If so I think I will just fix it in #5117. I actually noticed this bug as enabling the filestore broke id-hashes.

@whyrusleeping
Copy link
Member

@kevina that seems right. Please do the fix in a separate PR.

kevina added a commit that referenced this issue Jun 15, 2018
Closes #5121.

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@ghost ghost assigned kevina Jun 15, 2018
@ghost ghost added the status/in-progress In progress label Jun 15, 2018
@kevina
Copy link
Contributor Author

kevina commented Jun 15, 2018

@whyrusleeping see #5122.

@ghost ghost removed the status/in-progress In progress label Jun 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

2 participants