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

ipfs repo gc fails with Error: unrecognized object type: %!s(uint64=114) #3553

Closed
mib-kd743naq opened this issue Dec 30, 2016 · 19 comments
Closed
Assignees
Labels
kind/bug A bug in existing code (including security flaws) topic/repo Topic repo
Milestone

Comments

@mib-kd743naq
Copy link
Contributor

Version information:

go-ipfs version: 0.4.5-dev-99d72c2
Repo version: 4
System version: amd64/linux
Golang version: go1.7.4

Type:

Bug

Priority:

P1

Description:

I have maintained the same repository since I started playing with ipfs ~4 months ago. An attempt to clean up the repository currently fails like so:

~$ ipfs repo gc; echo $?
Error: unrecognized object type: %!s(uint64=114)
1

The last time I used gc (and it worked) was about a month ago, I have gone through probably 3 versions from git since.

I am not sure how to provide a backtrace/further diag for this, please advise.

@ghost
Copy link

ghost commented Dec 30, 2016

This is probably coming from decodeBlock() in merkledag/merkledag.go. You can change the respective line to the following, to see which node is the problem.

	default:
		return nil, fmt.Errorf("has unrecognized object type: %s in %s", c.Type(), c.String())

Ideally GC wouldn't abort just because it fails to read a block. This condition should be a warning at most.

@ghost
Copy link

ghost commented Dec 30, 2016

GC should skip these blocks -- it can't figure out whether they're okay to delete, and what's below it.

@mib-kd743naq
Copy link
Contributor Author

@lgierth with your change:

~$ ipfs repo gc
Error: unrecognized object type: %!s(uint64=114) in zdvgq3iFsC6r8Luprd25sKjXZYDTBtP3f1fgvkkGU4XKaSkcL
~$ ipfs block get zdvgq3iFsC6r8Luprd25sKjXZYDTBtP3f1fgvkkGU4XKaSkcL | hexdump -C
00000000  30 31 32 33 34 35                                 |012345|

That "block" is from when I was experimenting with the put API, checking how it reacts to garbage. Definitely a bug

@ghost
Copy link

ghost commented Dec 30, 2016

Thanks for your manual fuzzing, very much appreciated :)

@ghost ghost added kind/bug A bug in existing code (including security flaws) topic/repo Topic repo labels Dec 30, 2016
@jbenet
Copy link
Member

jbenet commented Dec 31, 2016

ohhh cool. @mib-kd743naq can you repro it with a sharness test case? if it's a block you put with the put api I would imagine you should be able to do it with a brand new repo. would be super helpful. thanks for finding this!

@mib-kd743naq
Copy link
Contributor Author

@lgierth this issue is more convoluted than I originally thought. Currently this is how things behave:

~$ echo -n 012345 | ipfs block put --format=raw 
zb2rhYwZB3fV3pD6QMaaBNbpEo9JfJ4nZYPd7uvcjdj5SHPAL

But it differs from the original problematic block of zdvgq3iFsC6r8Luprd25sKjXZYDTBtP3f1fgvkkGU4XKaSkcL by a single byte, due to this retroactive change

Generally this situation can never happen, but in practice it clearly does. The code should be hardened against this somehow...

@whyrusleeping
Copy link
Member

@mib-kd743naq thanks for reporting this, we should probably treat unrecognized blocks as 'raw' for all intents and purposes... though ideally we don't get unrecognized blocks in our repos

@Kubuxu Kubuxu added this to the ipfs 0.4.5 milestone Dec 31, 2016
@kevina kevina self-assigned this Jan 3, 2017
@kevina
Copy link
Contributor

kevina commented Jan 3, 2017

I'll look into fixing this.

@kevina
Copy link
Contributor

kevina commented Jan 4, 2017

If the unrecognized object is pinned indirectly or recursively it might not be safe to continue, as the object could possible contain links that we don't know how to read so we won't be able to figure out what is below it. I apologize if I am mistaken and this situation is impossible.

What I can likely do is complete the first stage of the gc (getting the ColoredSet) and collect all pinned objects that can't be read for what ever reason and output a complete list so the user can manually deal with the problem. This might also help with figuring out what is going on with #3453.

If on the other hand, the error is happening when trying to delete an unpinned block (or even just iterating over the set of the blocks in the blockstore) then we should continue. Right now it looks like when we can't delete a block we log a debug message and just return without necessary reporting an error.

I think I know how to reproduce this bug and will test for both the pinned and unpinned case and make sure we do something sensible` in both case.

@kevina kevina modified the milestones: ipfs 0.4.6, ipfs 0.4.5 Jan 4, 2017
@kevina
Copy link
Contributor

kevina commented Jan 4, 2017

Bumping to 0.4.6, after taking with @whyrusleeping, as the fix is non-trivial.

@ghost
Copy link

ghost commented Jan 5, 2017

What I can likely do is complete the first stage of the gc (getting the ColoredSet) and collect all pinned objects that can't be read for what ever reason and output a complete list so the user can manually deal with the problem. This might also help with figuring out what is going on with #3453.

Can't this be simplified to just printing a warning and skipping that block?

@Kubuxu
Copy link
Member

Kubuxu commented Jan 5, 2017

If it is not reffed by any other block, it should be?

@kevina
Copy link
Contributor

kevina commented Jan 5, 2017

This was bumped to the next release. I will have a better idea of where the problem is for this bug once I investigate, which I will likely do in a few weeks.

@whyrusleeping
Copy link
Member

While there is no official fix for this yet, I do have an experimental (Aka, works on my machines) program that should resolve these issues: https://github.com/whyrusleeping/repofix

I recommend making a backup of your ipfs dir if you can before running that, it shouldnt have any problems, but I always err on the safest side possible when it comes to user data.

@whyrusleeping whyrusleeping modified the milestones: Ipfs 0.4.7, ipfs 0.4.6 Feb 14, 2017
@kevina
Copy link
Contributor

kevina commented Feb 15, 2017

I can work on this. Do we have a clear test case of the problem?

@kevina kevina added the status/in-progress In progress label Feb 17, 2017
@kevina
Copy link
Contributor

kevina commented Feb 17, 2017

Okay, since I don't have anything else pressing to do I am going to start to work on this. I will basically work to make the garbage collector as resilient as it is safe to do so, and to report any errors in as clear and helpful manor as possible to make fixing any problems easy.

@whyrusleeping
Copy link
Member

@kevina sounds good to me, make sure to do so in small testable changesets :)

@kevina
Copy link
Contributor

kevina commented Feb 21, 2017

#3712 should help with this, it still needs testing though

@whyrusleeping whyrusleeping modified the milestones: Ipfs 0.4.8, Ipfs 0.4.7 Mar 11, 2017
@whyrusleeping
Copy link
Member

#3712 was merged. It doesnt solve these errors, but it makes them a lot less cryptic and easy to deal with

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) topic/repo Topic repo
Projects
None yet
Development

No branches or pull requests

5 participants