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

2 to 3 #7

Merged
merged 4 commits into from
Dec 19, 2015
Merged

2 to 3 #7

merged 4 commits into from
Dec 19, 2015

Conversation

whyrusleeping
Copy link
Member

2 to 3 ipfs fsrepo migration, mostly works. Probably could use a CR and some testing.

@whyrusleeping
Copy link
Member Author

I had to vendor most of ipfs to get this to work.

@jbenet jbenet mentioned this pull request May 21, 2015
52 tasks
@tv42
Copy link

tv42 commented May 22, 2015

Please don't commit vi swap files.

@whyrusleeping
Copy link
Member Author

thats what i get for git add ipfs-2-to-3

@tv42
Copy link

tv42 commented May 22, 2015

That's what you get for not having a good ~/.config/git/gitignore.

@whyrusleeping whyrusleeping mentioned this pull request May 26, 2015
49 tasks
@whyrusleeping whyrusleeping mentioned this pull request Jun 2, 2015
58 tasks
@jbenet
Copy link
Member

jbenet commented Jun 7, 2015

@whyrusleeping i think this needs to be redone after your PR to fix the import graph to not import all of ipfs. (adds tons of noise etc).

@whyrusleeping
Copy link
Member Author

@jbenet ready for some review here. I'll write a test for this when i get a little more time.

I also will squash these commits down whenever you want, theres a lot of noise...

@jbenet
Copy link
Member

jbenet commented Dec 9, 2015

the vendoring is making it really hard to review. not sure best way to do it

@whyrusleeping
Copy link
Member Author

yeah... this is pretty frustrating.

@jbenet
Copy link
Member

jbenet commented Dec 11, 2015

Comments above. Pinning migration looks ok to me.

But we really, really need to test this one, maybe even with sharness, and ideally with a large repo with lots of data, some subet of pins (that has lots of recursive and direct pins, some that overlap, and certainly that dont fill the whole content).

making sure this is correct is critical, as getting this wrong would lose user data!!! and mean disaster

cc @chriscool for sharnessifying help if we go that route

@jbenet
Copy link
Member

jbenet commented Dec 11, 2015

@whyrusleeping thanks for separating out the vendor commit

@chriscool
Copy link
Contributor

Yeah I can add sharness tests to this. I wonder if the tests should use docker and ipfs-update.

@chriscool
Copy link
Contributor

@whyrusleeping do you have ideas about how it should be tested?

@whyrusleeping
Copy link
Member Author

i'm fine having this tested with or without docker, my main idea for testing this goes something like:

  • download ipfs v0.3.10
  • init with that binary
  • A: generate a set of files that get added and pinned
  • B: generate a set of files that get added and unpinned
  • C: generate a set of directory trees that get added
  • D: generate a set of directory trees that have their subtrees added as well (pin root recursive, and some subtree)
  • E: generate some files that have their root directly pinned, and nothing else
  • run fs-repo-migration
  • download 0.4.0 binary
  • verify each of the above

@chriscool
Copy link
Contributor

Ok thanks! I will take a look at that.

You are ok with using ipfs-update?

@jbenet
Copy link
Member

jbenet commented Dec 12, 2015

it may be nice to start with the very first version, and ratchet up all the way, and all the way back down. to ensure all the migrations continue working. :) basically:

(first) for each repo version:

  • download a version of go-ipfs corresponding to that repo

for each repo version transition (A->B), both going up and going down:

  • (1) ipfs init (with binary known to work with A)
  • (2) exercise ipfs repo with lots of data + an involved workload:
    • add lots of files, some large. exercise pinset + gc, pin + unpin various nodes, directly or recursively. get lots of overlap. finish with significant unpinned content (which would go away with a repo gc) and with various recursive pins that overlap
    • https://github.com/jbenet/go-random-files may be useful here
  • (3) verify (a) content exists or does not exist in repo as expected, (b) pinset values work as expected (ie the right {recursive, direct, and indirect} pins)
  • (4) attempt to use binary known to work with B (should get repo version error)
  • (5) migrate repo from A to B (either up, or down)
  • (5) verify the same things again. (now with binary known to work with B)

@jbenet
Copy link
Member

jbenet commented Dec 12, 2015

maybe we can start with all that for 2 version transitions:

test transition 2-to-3
test transition 3-to-2

repo version 2 with go-ipfs@0.3.10
repo version 3 with go-ipfs@0.4.0

@chriscool
Copy link
Contributor

Ok, so I can use ipfs-update, if it can install 0.3.10 and 0.4.0.

But according to ipfs/ipfs-update#7 it cannot yet install 0.3.10.

@whyrusleeping
Copy link
Member Author

@jbenet this has been updated and is RFCR/Merge

@ghost
Copy link

ghost commented Dec 19, 2015

Yeah let's merge it so we can get it into folks' hands -- we can get more people testing dev040 if migration is easier

@whyrusleeping
Copy link
Member Author

Cool, lets merge it and get some feedback before we 'ship' it

whyrusleeping added a commit that referenced this pull request Dec 19, 2015
@whyrusleeping whyrusleeping merged commit 0849bab into master Dec 19, 2015
@whyrusleeping whyrusleeping deleted the 2-to-3 branch December 19, 2015 05:37
@whyrusleeping
Copy link
Member Author

@chriscool let me know any progress you make towards testing here

@chriscool
Copy link
Contributor

@whyrusleeping I'd like to finish testing ipfs-update first because I'd like to use it to test it this.

So it could help if you could have a look at the failures in: ipfs/ipfs-update#12

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.

4 participants