S3 datastore support #1261

tv42 · 2015-05-20T22:07:48Z

For your reviewing pleasure, do not merge yet. Sorry for the noise in the pull request, this is on top of the pin branch, start reading from the most recent commit with "pin:" in the commit message. Once pinning is fully merged, I'll rebase this and ask for a merge, but for now living on top of that branch causes less noise in the datastore.

Note that S3 objects names are hex, because datastore keys can be binary. So that means S3 objects like 2f626c6f636b732f122014d8c7e2382fe98c052b96b7a86daa9b176fde6324cbfdd37314617dc1058a7d.

whyrusleeping · 2015-05-20T22:13:11Z

@jbenet dont you dare git push each this one, i want to run other tests this month.

whyrusleeping · 2015-05-22T20:26:48Z

@tv42 do you have any thoughts on query for the s3datastore? thats gonna be the hard part i think.

Also, how hard would it be to run a local 's3' server for testing this code?

Most of this looks good so far.

tv42 · 2015-05-22T21:02:37Z

@whyrusleeping I don't expect S3 to bring any special difficulties to Query. I do want to whack the Query API with a mallet; whether that happens as part of the continuation of this or not depends on whether I come up with a good enough replacement API soon enough. If not, s3 Query will look a lot like flatfs Query, except it could support more (unused) features.

I'm not aware of any "tiny s3 clones" worth recommending, and didn't want to start writing one on a whim. Making a true S3 clone is ridiculously hard (been there; https://github.com/ceph/s3-tests/ ), though this code uses a small enough of a fraction of features that httptest.Server might be plenty enough.

Also, remember that it's currently only 70 lines of code and 83 lines to configure that (most of that validating the JSON, for better error reporting), and maybe 60-100 LoC for a simple Query implementation. And it's pretty much all very simple code.

Perhaps some of the sharness stuff could be run in a special mode, where they talk to actual AWS S3?

whyrusleeping · 2015-05-22T22:13:13Z

Perhaps some of the sharness stuff could be run in a special mode, where they talk to actual AWS S3?

Maybe... that's probably okay to do if we make sure they only run when a given env var (or set of env vars) is set

jbenet · 2015-06-07T22:39:33Z

repo/config/datastore.go

+// All values of DSOpener must have a JSON representation that is
+// semantically equivalent to the Datastore section of the config
+// file that they were instantiated by, including the "Type" key.
+type DSOpener interface {


good idea 👍

jbenet · 2015-06-07T22:56:03Z

@tv42 this PR is great. +1 everything LGTM. sorry for the huge delay on my end.

I do want to whack the Query API with a mallet; whether that happens as part of the continuation of this or not depends on whether I come up with a good enough replacement API soon enough. If not, s3 Query will look a lot like flatfs Query, except it could support more (unused) features.

Either sounds good to me.

though this code uses a small enough of a fraction of features that httptest.Server might be plenty enough.

Yeah i think this works for normal tests. And, there's many S3 mock things out there that already implement the exact API for us (not that it's complicated...)

Perhaps some of the sharness stuff could be run in a special mode, where they talk to actual AWS S3?

Maybe... that's probably okay to do if we make sure they only run when a given env var (or set of env vars) is set

Yeah, we can have a special set of sharness tests that can be run against S3. I wouldn't want to put this on CI for all PRs. (could make a simple side-repo with the right config that pulls go-ipfs and tests on S3, and make sure to pull to this repo before merging to stable. would need a way to make credentials private or whatever)

@whyrusleeping

@jbenet dont you dare git push each this one, i want to run other tests this month.

GitCop · 2015-07-15T02:35:18Z

There were the following issues with your Pull Request

Commit: ef34768
- Invalid signoff. Commit message must end with License: MIT
  Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue

This message was auto-generated by https://gitcop.com

GitCop · 2015-07-15T02:46:01Z

There were the following issues with your Pull Request

Commit: ef34768
- Invalid signoff. Commit message must end with License: MIT
  Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue

This message was auto-generated by https://gitcop.com

tv42 · 2015-07-15T02:51:06Z

Gitcop is wrong (ipfs/infra#46), and my commits have been amended to have the right trailer.

Unit tests pass, sharness passes.

This is on top of dev0.4.0 (with a fix for a random bug I found in dev0.4.0) and ready to roll! Paging @jbenet @whyrusleeping

tv42 · 2015-07-15T03:02:35Z

I have no idea about those circleci & travis errors in https://circleci.com/gh/ipfs/go-ipfs/640 https://travis-ci.org/ipfs/go-ipfs/jobs/71014668 but none of that happens locally and none of it seems to be related to these changes.

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 <[email protected]>

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 <[email protected]>

License: MIT Signed-off-by: Jeromy <[email protected]>

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 <[email protected]>

License: MIT Signed-off-by: Jeromy <[email protected]>

Pinner had method GetManual that returned a ManualPinner, so every Pinner had to implement ManualPinner anyway. License: MIT Signed-off-by: Jeromy <[email protected]>

License: MIT Signed-off-by: Jeromy <[email protected]>

Platform-dependent behavior is not nice, and negative refcounts are not very useful. License: MIT Signed-off-by: Jeromy <[email protected]>

License: MIT Signed-off-by: Jeromy <[email protected]>

GitCop · 2015-07-15T15:24:32Z

There were the following issues with your Pull Request

Commit: 82afdb1
- Invalid signoff. Commit message must end with License: MIT
  Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue

This message was auto-generated by https://gitcop.com

tv42 · 2015-07-15T15:24:58Z

Rebased again to get on top of latest dev0.4.0.

The generated file went through some changes because of differing go-bindata versions. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

This gives us a clean slate for the new code, avoiding leftovers. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

Earlier, it also checked checked the leveldb directory. That part added no crash safety to the application, and just hardcoded assumptions about the datastore. If anything, this should rely on the absolute last item created by fsrepo.Init, and there should be fsync guarantees about ordering. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

…m for S3 License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

…"private" License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

To test it, set up an S3 bucket (in an AWS region that is not US Standard, for read-after-write consistency), run `ipfs init`, then edit `~/.ipfs/config` to say "Datastore": { "Type": "s3", "Region": "us-west-1", "Bucket": "mahbukkit", "ACL": "private" }, with the right values. Set `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` in the environment and you should be able to run `ipfs add` and `ipfs cat` and see the bucket be populated. No automated tests exist, unfortunately. S3 is thorny to simulate. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

GitCop · 2015-07-15T15:39:17Z

There were the following issues with your Pull Request

Commit: 82afdb1
- Invalid signoff. Commit message must end with License: MIT
  Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue

This message was auto-generated by https://gitcop.com

tv42 · 2015-07-16T20:00:45Z

Closing in favor of #1488 because that one targets dev0.4.0. This one is from before the dev branch existed.

jbenet added the status/in-progress In progress label May 20, 2015

tv42 mentioned this pull request May 22, 2015

Sprint H ipfs/team-mgmt#9

Closed

52 tasks

whyrusleeping added codereview and removed status/in-progress In progress labels May 31, 2015

jbenet reviewed Jun 7, 2015
View reviewed changes

jbenet mentioned this pull request Jun 26, 2015

cleanup unused packages #1427

Merged

tv42 force-pushed the s3 branch from f857e0d to ba6686b Compare July 15, 2015 02:35

tv42 force-pushed the s3 branch from ba6686b to dda3c6a Compare July 15, 2015 02:45

tv42 mentioned this pull request Jul 15, 2015

Gitcop breaks on multiple Signed-Off-Bys ipfs/infra#46

Closed

tv42 changed the title ~~S3 datastore support (for review, don't merge yet)~~ S3 datastore support Jul 15, 2015

tv42 added 11 commits July 14, 2015 21:40

pin: Guard against callers causing refcount underflow

feaeb78

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 <[email protected]>

sharness: fusermount -u is the documented way to unmount FUSE on Linux

b168645

License: MIT Signed-off-by: Jeromy <[email protected]>

pin: unexport NewIndirectPin, it's not useful and not used elsewhere

1d43c8e

License: MIT Signed-off-by: Jeromy <[email protected]>

blocks/set: Remove now-unused NewDBWrapperSet

3678749

License: MIT Signed-off-by: Jeromy <[email protected]>

Simplify Pinner interface by folding ManualPinner into Pinner

950b73b

Pinner had method GetManual that returned a ManualPinner, so every Pinner had to implement ManualPinner anyway. License: MIT Signed-off-by: Jeromy <[email protected]>

pin: Remove dead code

7749aec

License: MIT Signed-off-by: Jeromy <[email protected]>

pin: Remove double bookkeeping of refcount keys

b6acf81

License: MIT Signed-off-by: Jeromy <[email protected]>

Use uint64 for indirect pin refcounts

ff98e1c

Platform-dependent behavior is not nice, and negative refcounts are not very useful. License: MIT Signed-off-by: Jeromy <[email protected]>

Typo

4bd8a26

License: MIT Signed-off-by: Jeromy <[email protected]>

whyrusleeping added 6 commits July 15, 2015 08:03

break up GC logic

4032112

License: MIT Signed-off-by: Jeromy <[email protected]>

addressing comments from CR

a99982b

License: MIT Signed-off-by: Jeromy <[email protected]>

pin rm fails appropriately for indirect pins

09d6802

License: MIT Signed-off-by: Jeromy <[email protected]>

dont use searchset for indirect pin checking

e6dd70a

License: MIT Signed-off-by: Jeromy <[email protected]>

allow multistream to have zero rtt stream opening

b5b27f1

License: MIT Signed-off-by: Jeromy <[email protected]>

update multistream naming of lazyconn

0636f3c

License: MIT Signed-off-by: Jeromy <[email protected]>

tv42 force-pushed the s3 branch from dda3c6a to 651a0cd Compare July 15, 2015 15:24

tv42 added 13 commits July 15, 2015 08:36

gofmt generated assets

3c5be9b

The generated file went through some changes because of differing go-bindata versions. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

Remove dead code

f606a23

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

core tests: Stop assuming internals of Config

3286ee5

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

sharness: Stop assuming leveldb Datastore

5a3de50

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

fsrepo/serialize tests: Stop assuming internals of Config

2c963ee

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

Remove Config file section "Datastore", it's not used

09a5d93

This gives us a clean slate for the new code, avoiding leftovers. License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

fsrepo: Refactor to extract datastore internals

bcc3149

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

Implement pluggable Datastore types, with nothing implemented yet

1521599

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

thirdparty/s3-datastore: Datastore keys can be binary, hex encode the…

3b89846

…m for S3 License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

thirdparty/s3-datastore: Let caller set ACL, change default to safer …

62b5780

…"private" License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

Record datastore metrics for non-default datastores

fc949cb

License: MIT Signed-off-by: Tommi Virtanen <[email protected]>

tv42 force-pushed the s3 branch from 651a0cd to fc949cb Compare July 15, 2015 15:39

tv42 mentioned this pull request Jul 16, 2015

S3 0.4.0 #1488

Merged

tv42 closed this Jul 16, 2015

jbenet removed the codereview label Jul 16, 2015

almereyda mentioned this pull request Dec 30, 2015

Update S3 code to use s3gof3r #2016

Closed

Kubuxu deleted the s3 branch February 27, 2017 20:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

S3 datastore support #1261

S3 datastore support #1261

tv42 commented May 20, 2015

whyrusleeping commented May 20, 2015

whyrusleeping commented May 22, 2015

tv42 commented May 22, 2015

whyrusleeping commented May 22, 2015

jbenet Jun 7, 2015

jbenet commented Jun 7, 2015

GitCop commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 15, 2015

tv42 commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 16, 2015

S3 datastore support #1261

S3 datastore support #1261

Conversation

tv42 commented May 20, 2015

whyrusleeping commented May 20, 2015

whyrusleeping commented May 22, 2015

tv42 commented May 22, 2015

whyrusleeping commented May 22, 2015

jbenet Jun 7, 2015

Choose a reason for hiding this comment

jbenet commented Jun 7, 2015

GitCop commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 15, 2015

tv42 commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 15, 2015

GitCop commented Jul 15, 2015

tv42 commented Jul 16, 2015