more configurable datastore configs #3575

whyrusleeping · 2017-01-08T11:31:32Z

whyrusleeping · 2017-03-11T01:00:08Z

@Kubuxu could I get some early review on this?

Kubuxu · 2017-03-17T19:40:38Z

This branch allows for usage for new experimental datastore (sbs). To use it you have to create new fresh repo.

ipfs init --empty-repo
ipfs config --json Datastore.Spec '{"mounts":[{"child":{"path":"blocks","type":"sbs"},"mountpoint":"/blocks","prefix":"flatfs.datastore","type":"measure"},{"child":{"compression":"none","path":"datastore","type":"levelds"},"mountpoint":"/","prefix":"leveldb.datastore","type":"measure"}],"type":"mount"}'

whyrusleeping · 2017-03-18T05:40:09Z

@Kubuxu nice work so far :)

As an aside, we should look at adding json omitempty tags to the config fields we're deprecating, and start planning a migration path. I think we can pretty easily migrate people from the current format forward into the new format with an fs-repo-migration. At the same time we should adjust for 'new defaults' like the CORS thing that people who init'ed with old nodes might not have.

Perhaps with this migration, we even move the private key out of the config??

whyrusleeping · 2017-03-18T05:41:16Z

Idea:

Maybe try and ship this code in 0.4.8 (perhaps minus the sbs stuff for now) but keep the default config the same. Then in 0.4.9 we can have a migration to update everyone to the new format

whyrusleeping · 2017-05-08T03:23:01Z

sigh this keeps slipping. I really want to get it in soon...

kevina · 2017-05-08T03:38:13Z

@whyrusleeping is there anything I can do to help here?

whyrusleeping · 2017-05-08T17:21:03Z

@kevina Yeah, if you want to take this over. Drop the 'sbs' commit from @Kubuxu, and add some tests. That would be wonderful

kevina · 2017-05-08T19:33:18Z

@whyrusleeping @Kubuxu Should I should drop the "--wip--" commit also?

kevina · 2017-05-08T20:29:38Z

repo/config/datastore.go

+type FlatDS struct {
+	Path      string
+	ShardFunc string
+	Sync      bool


So we changed this from NoSync to Sync, was this intentional? Is the default now not to enable Sync?

And and closer inspection it appears that this data structure isn't even used...

kevina · 2017-05-08T20:30:32Z

repo/config/init.go

+					"child": map[string]interface{}{
+						"type":      "flatfs",
+						"path":      "blocks",
+						"nosync":    false,


I might be misunderstanding something, but should't this be "sync": true now?

yeah, this should probably be sync true now

except we still check for "nosync" still in the code, one sec, I leave a comment by that code.

kevina · 2017-05-08T20:31:59Z

I dropped the last two commits, they are saved on feat/datastore-configs-wip for now.

kevina · 2017-05-08T21:15:13Z

A few notes before I am ready to finish this:

The Datastore specific config structs do not appear to be used. Should we make use of them or delete them. Right now the FlatDS stuct is in accuance as the code is still checking for "nosync" even the the FlatDS uses sync.
The config code doesn't seam to check for invalid keys (even outside the Datastore specific code) is this something we want to keep as is.
I will add "json omitempty tag" to the fields marked depicted as per @whyrusleeping suggestion.

whyrusleeping · 2017-05-09T02:12:06Z

@kevina yeah, i think we can remove the datastore specific structs. I wrote those before i figured out how this was gonna work. We can just use the interface map stuff.

Checking for 'disallowed keys' might be nice, though not super simple to do. If you want to, go ahead and add this checking, otherwise leave it unchecked.

your '3' SGTM

kevina · 2017-05-09T02:19:27Z

repo/fsrepo/datastores.go

+		return nil, err
+	}
+
+	return flatfs.CreateOrOpen(p, shardFun, params["nosync"].(bool))


Okay so here is where we check for "nosync" except that CreateOrOpen parameter is to sync, no "nosync" so this is a bug as written and I am okay with changing it to "sync".

kevina · 2017-08-25T23:53:26Z

@whyrusleeping oops, forgot about that will push a fix shortly and update the conversion code

kevina · 2017-08-26T00:19:12Z

@whyrusleeping I am not sure I agree that "Spec" should be lowercase, the config now looks like this:

{
    "StorageMax": "10GB",
    "StorageGCWatermark": 90,
    "GCPeriod": "1h",
    "spec": {
      "mounts": [
        {
          "child": {
            "compression": "none",
            "path": "datastore",
            "type": "levelds"
          },
          "mountpoint": "/",
          "prefix": "leveldb.datastore",
          "type": "measure"
        },
        {
          "child": {
            "path": "blocks",
            "shardFunc": "/repo/flatfs/shard/v1/next-to-last/2",
            "sync": true,
            "type": "flatfs"
          },
          "mountpoint": "/blocks",
          "prefix": "flatfs.datastore",
          "type": "measure"
        }
      ],
      "type": "mount"
    },
    "HashOnRead": false,
    "BloomFilterSize": 0
}

If you think that is better I will go ahead and push the change and fix the conversion code.

kevina · 2017-08-26T00:21:04Z

I don't think it is better because all the other fields under the Datastore config are title case (i.e capitalized) with just "spec" being lower case.

whyrusleeping · 2017-08-26T17:46:40Z

@kevina hrm.... yeah... youre right. We definitely should (at some point in the future) make the config look nicer.

kevina · 2017-08-26T17:49:34Z

So leave things as they are for now, with "Spec" being capitalized?

whyrusleeping · 2017-08-28T22:10:16Z

@kevina yeah.

This PR is RFM, would be nice to get an approval from @Kubuxu and @Stebalien, but once we have a release ready for the fs-repo-migrations (pending some config changes @lgierth wanted to add) we can ship this finally :)

Stebalien

LGTM (sorry, I thought I checked the approve checkbox before).

License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com> make things super customizable License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com> better json format License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com> Migrate to new flatfs License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

…ault License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>

whyrusleeping added status/in-progress In progress labels Jan 8, 2017

whyrusleeping force-pushed the feat/datastore-configs branch from 4d1bef1 to 0c224c7 Compare January 8, 2017 12:05

whyrusleeping added this to the Ipfs 0.4.7 milestone Feb 17, 2017

whyrusleeping force-pushed the feat/datastore-configs branch 2 times, most recently from 06e62c3 to 10eabf3 Compare February 24, 2017 22:35

whyrusleeping mentioned this pull request Feb 24, 2017

Add support for s3 backed ipfs repo #3726

Closed

whyrusleeping requested a review from Kubuxu March 11, 2017 01:00

whyrusleeping modified the milestones: Ipfs 0.4.8, Ipfs 0.4.7 Mar 11, 2017

Kubuxu force-pushed the feat/datastore-configs branch from 10eabf3 to b962c86 Compare March 17, 2017 11:56

whyrusleeping modified the milestones: Ipfs 0.4.9, Ipfs 0.4.8 Mar 24, 2017

whyrusleeping modified the milestones: Ipfs 0.4.10, Ipfs 0.4.9 May 8, 2017

kevina force-pushed the feat/datastore-configs branch from a23eb2a to 4cd09ce Compare May 8, 2017 20:21

kevina reviewed May 8, 2017

View reviewed changes

kevina reviewed May 9, 2017

View reviewed changes

Stebalien approved these changes Aug 31, 2017

View reviewed changes

whyrusleeping and others added 15 commits September 3, 2017 14:08

cleanup and bug fixes

0f8b603

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

bump repo version, remove support for old config

2df7727

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

address p.r. comments

ee935ab

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

add tests for new datastore configuration

5a8c171

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Use DatastoreConfig abstraction to create datastores.

1f97170

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Implement DiskId()

5027b4b

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

add basic tests for Spec config change

7b5027f

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

write out full spec instead of just the disk id

3bbe065

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Use the json repr. of the minimal config as the DiskId.

d64ab3c

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Write the spec file during initialization only.

ce62bed

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Error out on unknown levelds compression rather than assuming the def…

a99448f

…ault License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Add documentation.

6438fc7

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Address p.r. feedback

1f8723d

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

Sort mountpoints.

19730d4

License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>

whyrusleeping force-pushed the feat/datastore-configs branch from 7e4bcae to 19730d4 Compare September 3, 2017 21:09

whyrusleeping and others added 3 commits September 3, 2017 15:20

use dists with new migrations

ef5e94a

License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>

fix EOF quoting

0b0ddef

License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>

fix t0024 on osx

2c30002

License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>

whyrusleeping force-pushed the feat/datastore-configs branch from ea4ce27 to 2c30002 Compare September 4, 2017 01:43

whyrusleeping merged commit cbd13b6 into master Sep 4, 2017

whyrusleeping deleted the feat/datastore-configs branch September 4, 2017 02:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

more configurable datastore configs #3575

more configurable datastore configs #3575

whyrusleeping commented Jan 8, 2017 •

edited by kevina

Loading

whyrusleeping commented Mar 11, 2017

Kubuxu commented Mar 17, 2017 •

edited

Loading

whyrusleeping commented Mar 18, 2017

whyrusleeping commented Mar 18, 2017

whyrusleeping commented May 8, 2017

kevina commented May 8, 2017

whyrusleeping commented May 8, 2017

kevina commented May 8, 2017

kevina May 8, 2017

kevina May 8, 2017

kevina May 8, 2017

whyrusleeping May 9, 2017

kevina May 9, 2017 •

edited

Loading

kevina commented May 8, 2017 •

edited

Loading

kevina commented May 8, 2017

whyrusleeping commented May 9, 2017

kevina May 9, 2017

kevina commented Aug 25, 2017

kevina commented Aug 26, 2017

kevina commented Aug 26, 2017 •

edited

Loading

whyrusleeping commented Aug 26, 2017

kevina commented Aug 26, 2017 •

edited

Loading

whyrusleeping commented Aug 28, 2017

Stebalien left a comment

more configurable datastore configs #3575

more configurable datastore configs #3575

Conversation

whyrusleeping commented Jan 8, 2017 • edited by kevina Loading

whyrusleeping commented Mar 11, 2017

Kubuxu commented Mar 17, 2017 • edited Loading

whyrusleeping commented Mar 18, 2017

whyrusleeping commented Mar 18, 2017

whyrusleeping commented May 8, 2017

kevina commented May 8, 2017

whyrusleeping commented May 8, 2017

kevina commented May 8, 2017

kevina May 8, 2017

Choose a reason for hiding this comment

kevina May 8, 2017

Choose a reason for hiding this comment

kevina May 8, 2017

Choose a reason for hiding this comment

whyrusleeping May 9, 2017

Choose a reason for hiding this comment

kevina May 9, 2017 • edited Loading

Choose a reason for hiding this comment

kevina commented May 8, 2017 • edited Loading

kevina commented May 8, 2017

whyrusleeping commented May 9, 2017

kevina May 9, 2017

Choose a reason for hiding this comment

kevina commented Aug 25, 2017

kevina commented Aug 26, 2017

kevina commented Aug 26, 2017 • edited Loading

whyrusleeping commented Aug 26, 2017

kevina commented Aug 26, 2017 • edited Loading

whyrusleeping commented Aug 28, 2017

Stebalien left a comment

Choose a reason for hiding this comment

whyrusleeping commented Jan 8, 2017 •

edited by kevina

Loading

Kubuxu commented Mar 17, 2017 •

edited

Loading

kevina May 9, 2017 •

edited

Loading

kevina commented May 8, 2017 •

edited

Loading

kevina commented Aug 26, 2017 •

edited

Loading

kevina commented Aug 26, 2017 •

edited

Loading