fix(fsrepo/datastore) allow goroutines to share the datastore.
doh! I forgot to make sure leveldb is only opened once. thanks for catching this @mappum * You may be wondering why we don't just share pointers to FSRepos. We want to manage the lifecycle of the FSRepo by tracking its `state`. Thus each FSRepo/goroutine requires private instance variables. For this reason, each `fsrepo.At(p)` caller must get its own goroutine. * There's a test in `fsrepo` because callers desire the ability to Open from multiple goroutines. There's a test in `component` because this is where the actual work needs to go in order to provide the desired contract. If the `component` package moves, the assurances need to move along with it. cc @whyrusleeping @jbenet side note: there are a couple packages in FSRepo that it might be worthwhile to extract once the dust settles on this feature-set.
Brian Tiger Chow committed
Jan 14, 2015 at 13:18 UTC
bc76d2e526014590dc5f77d5a2e9ebfa77315ea9
3 files changed
+106
-14
repo/fsrepo/component/datastore.go
+62
-14
@@ -1,19 +1,34 @@
1
package component
2
3
import (
4
+ "errors"
5
+ "sync"
6
+
7
datastore "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore"
8
levelds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/leveldb"
9
ldbopts "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/syndtr/goleveldb/leveldb/opt"
10
config "github.com/jbenet/go-ipfs/repo/config"
11
+ counter "github.com/jbenet/go-ipfs/repo/fsrepo/counter"
12
dir "github.com/jbenet/go-ipfs/repo/fsrepo/dir"
13
util "github.com/jbenet/go-ipfs/util"
14
ds2 "github.com/jbenet/go-ipfs/util/datastore2"
15
debugerror "github.com/jbenet/go-ipfs/util/debugerror"
16
)
17
14
-var _ Component = &DatastoreComponent{}
15
-var _ Initializer = InitDatastoreComponent
16
-var _ InitializationChecker = DatastoreComponentIsInitialized
18
+var (
19
+ _ Component = &DatastoreComponent{}
20
+ _ Initializer = InitDatastoreComponent
21
+ _ InitializationChecker = DatastoreComponentIsInitialized
22
+
23
+ dsLock sync.Mutex // protects openersCounter and datastores
24
+ openersCounter *counter.Openers
25
+ datastores map[string]ds2.ThreadSafeDatastoreCloser
26
+)
27
+
28
+func init() {
29
+ openersCounter = counter.NewOpenersCounter()
30
+ datastores = make(map[string]ds2.ThreadSafeDatastoreCloser)
31
+}
32
33
func InitDatastoreComponent(path string, conf *config.Config) error {
34
// The actual datastore contents are initialized lazily when Opened.
@@ -41,24 +56,57 @@ func DatastoreComponentIsInitialized(path string) bool {
56
}
57
58
// DatastoreComponent abstracts the datastore component of the FSRepo.
44
-// NB: create with makeDatastoreComponent function.
59
type DatastoreComponent struct {
46
- path string
47
- ds ds2.ThreadSafeDatastoreCloser
60
+ path string // required
61
+ ds ds2.ThreadSafeDatastoreCloser // assigned when repo is opened
62
}
63
64
+func (dsc *DatastoreComponent) SetPath(p string) { dsc.path = p }
65
+func (dsc *DatastoreComponent) Datastore() datastore.ThreadSafeDatastore { return dsc.ds }
66
+
67
// Open returns an error if the config file is not present.
68
func (dsc *DatastoreComponent) Open() error {
52
- ds, err := levelds.NewDatastore(dsc.path, &levelds.Options{
53
- Compression: ldbopts.NoCompression,
54
- })
55
- if err != nil {
56
- return err
69
+
70
+ dsLock.Lock()
71
+ defer dsLock.Unlock()
72
+
73
+ // if no other goroutines have the datastore Open, initialize it and assign
74
+ // it to the package-scoped map for the goroutines that follow.
75
+ if openersCounter.NumOpeners(dsc.path) == 0 {
76
+ ds, err := levelds.NewDatastore(dsc.path, &levelds.Options{
77
+ Compression: ldbopts.NoCompression,
78
+ })
79
+ if err != nil {
80
+ return errors.New("unable to open leveldb datastore")
81
+ }
82
+ datastores[dsc.path] = ds
83
+ }
84
+
85
+ // get the datastore from the package-scoped map and record self as an
86
+ // opener.
87
+ ds, dsIsPresent := datastores[dsc.path]
88
+ if !dsIsPresent {
89
+ // This indicates a programmer error has occurred.
90
+ return errors.New("datastore should be available, but it isn't")
91
}
92
dsc.ds = ds
93
+ openersCounter.AddOpener(dsc.path) // only after success
94
return nil
95
}
96
62
-func (dsc *DatastoreComponent) Close() error { return dsc.ds.Close() }
63
-func (dsc *DatastoreComponent) SetPath(p string) { dsc.path = p }
64
-func (dsc *DatastoreComponent) Datastore() datastore.ThreadSafeDatastore { return dsc.ds }
97
+func (dsc *DatastoreComponent) Close() error {
98
+
99
+ dsLock.Lock()
100
+ defer dsLock.Unlock()
101
+
102
+ // decrement the Opener count. if this goroutine is the last, also close
103
+ // the underlying datastore (and remove its reference from the map)
104
+
105
+ openersCounter.RemoveOpener(dsc.path)
106
+
107
+ if openersCounter.NumOpeners(dsc.path) == 0 {
108
+ delete(datastores, dsc.path) // remove the reference
109
+ return dsc.ds.Close()
110
+ }
111
+ return nil
112
+}
repo/fsrepo/component/datastore_test.go
new
+30
@@ -0,0 +1,30 @@
1
+package component
2
+
3
+import (
4
+ "io/ioutil"
5
+ "path/filepath"
6
+ "testing"
7
+
8
+ "github.com/jbenet/go-ipfs/repo/fsrepo/assert"
9
+)
10
+
11
+// swap arg order
12
+func testRepoPath(t *testing.T, path ...string) string {
13
+ name, err := ioutil.TempDir("", filepath.Join(path...))
14
+ if err != nil {
15
+ t.Fatal(err)
16
+ }
17
+ return name
18
+}
19
+
20
+func TestOpenMoreThanOnceInSameProcess(t *testing.T) {
21
+ t.Parallel()
22
+ path := testRepoPath(t)
23
+ dsc1 := DatastoreComponent{path: path}
24
+ dsc2 := DatastoreComponent{path: path}
25
+ assert.Nil(dsc1.Open(), t, "first repo should open successfully")
26
+ assert.Nil(dsc2.Open(), t, "second repo should open successfully")
27
+
28
+ assert.Nil(dsc1.Close(), t)
29
+ assert.Nil(dsc2.Close(), t)
30
+}
repo/fsrepo/fsrepo_test.go
+14
@@ -125,3 +125,17 @@ func TestDatastorePersistsFromRepoToRepo(t *testing.T) {
125
assert.Nil(r2.Close(), t)
126
assert.True(bytes.Compare(expected, actual) == 0, t, "data should match")
127
}
128
+
129
+func TestOpenMoreThanOnceInSameProcess(t *testing.T) {
130
+ t.Parallel()
131
+ path := testRepoPath("", t)
132
+ assert.Nil(Init(path, &config.Config{}), t)
133
+
134
+ r1 := At(path)
135
+ r2 := At(path)
136
+ assert.Nil(r1.Open(), t, "first repo should open successfully")
137
+ assert.Nil(r2.Open(), t, "second repo should open successfully")
138
+
139
+ assert.Nil(r1.Close(), t)
140
+ assert.Nil(r2.Close(), t)
141
+}