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

Define GCDatastore/CheckedDatastore interfaces #68

Merged
merged 4 commits into from
Oct 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gx/lastpubver
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.2.2: QmVSase1JP7cq9QkPT46oNwdp9pT6kBkG3oqS14y3QcZjG
1.3.0: QmTWw8RaXEE155hrm8GiMYAYoUrAMwi6qt72CBpzdeGq1s
21 changes: 21 additions & 0 deletions basic_ds.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,24 @@ func (d *LogDatastore) Close() error {
}
return nil
}

func (d *LogDatastore) Check() error {
if c, ok := d.child.(CheckedDatastore); ok {
return c.Check()
}
return nil
}

func (d *LogDatastore) Scrub() error {
if c, ok := d.child.(ScrubbedDatastore); ok {
return c.Scrub()
}
return nil
}

func (d *LogDatastore) CollectGarbage() error {
if c, ok := d.child.(GCDatastore); ok {
return c.CollectGarbage()
}
return nil
}
26 changes: 26 additions & 0 deletions datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,35 @@ var ErrBatchUnsupported = errors.New("this datastore does not support batching")
// implement to leverage type safety checks.
type ThreadSafeDatastore interface {
Datastore

IsThreadSafe()
}

// CheckedDatastore is an interface that should be implemented by datastores
// which may need checking on-disk data integrity.
type CheckedDatastore interface {
Datastore

Check() error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Scrub()? We also want to support datastores that can do error correction (where an error is returned iff the datastore is unrecoverable).

An open question is how do deal with partial failures. It would nice to be able to recover as much data as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check would be invoked by ipfs repo fsck, which for now only removes lock files.

Scrub() does make sense (it could be wired up to ipfs repo verify, to keep repo fsck fast). Does putting this into separate interface SGTY (keeping check for stuff like lock-files)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Yes, having a separate interface sounds good.

Just to make sure, I assume Check() will find and fix any errors related to daemon crashes (i.e., it doesn't just blindly remove the lock file)?


Side note: we should get rid of ipfs repo fsck; IPFS should do any recovery necessary on startup automatically.

}

// CheckedDatastore is an interface that should be implemented by datastores
// which want to provide a mechanism to check data integrity and/or
// error correction
type ScrubbedDatastore interface {
Datastore

Scrub() error
}

// GCDatastore is an interface that should be implemented by datastores which
// don't free disk space by just removing data from them
type GCDatastore interface {
Datastore

CollectGarbage() error
}

// Errors

// ErrNotFound is returned by Get, Has, and Delete when a datastore does not
Expand Down
21 changes: 21 additions & 0 deletions keytransform/keytransform.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,24 @@ func (t *transformBatch) Delete(key ds.Key) error {
func (t *transformBatch) Commit() error {
return t.dst.Commit()
}

func (d *ktds) Check() error {
if c, ok := d.child.(ds.CheckedDatastore); ok {
return c.Check()
}
return nil
}

func (d *ktds) Scrub() error {
if c, ok := d.child.(ds.ScrubbedDatastore); ok {
return c.Scrub()
}
return nil
}

func (d *ktds) CollectGarbage() error {
if c, ok := d.child.(ds.GCDatastore); ok {
return c.CollectGarbage()
}
return nil
}
15 changes: 14 additions & 1 deletion keytransform/keytransform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
ds "github.com/ipfs/go-datastore"
kt "github.com/ipfs/go-datastore/keytransform"
dsq "github.com/ipfs/go-datastore/query"
dstest "github.com/ipfs/go-datastore/test"
)

// Hook up gocheck into the "go test" runner.
Expand All @@ -35,7 +36,7 @@ func (ks *DSSuite) TestBasic(c *C) {
},
}

mpds := ds.NewMapDatastore()
mpds := dstest.NewTestDatastore(true)
ktds := kt.Wrap(mpds, pair)

keys := strsToKeys([]string{
Expand Down Expand Up @@ -88,6 +89,18 @@ func (ks *DSSuite) TestBasic(c *C) {

c.Log("listA: ", listA)
c.Log("listB: ", listB)

if err := ktds.Check(); err != dstest.TestError {
c.Errorf("Unexpected Check() error: %s", err)
}

if err := ktds.CollectGarbage(); err != dstest.TestError {
c.Errorf("Unexpected CollectGarbage() error: %s", err)
}

if err := ktds.Scrub(); err != dstest.TestError {
c.Errorf("Unexpected Scrub() error: %s", err)
}
}

func strsToKeys(strs []string) []ds.Key {
Expand Down
34 changes: 34 additions & 0 deletions mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package mount

import (
"errors"
"fmt"
"io"
"sort"
"strings"
Expand Down Expand Up @@ -251,3 +252,36 @@ func (mt *mountBatch) Commit() error {
}
return nil
}

func (d *Datastore) Check() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(datastore.CheckedDatastore); ok {
if err := c.Check(); err != nil {
return fmt.Errorf("checking datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}

func (d *Datastore) Scrub() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(datastore.ScrubbedDatastore); ok {
if err := c.Scrub(); err != nil {
return fmt.Errorf("scrubbing datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}

func (d *Datastore) CollectGarbage() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(datastore.GCDatastore); ok {
if err := c.CollectGarbage(); err != nil {
return fmt.Errorf("gc on datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}
20 changes: 20 additions & 0 deletions mount/mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/ipfs/go-datastore"
"github.com/ipfs/go-datastore/mount"
"github.com/ipfs/go-datastore/query"
dstest "github.com/ipfs/go-datastore/test"
)

func TestPutBadNothing(t *testing.T) {
Expand Down Expand Up @@ -371,3 +372,22 @@ func TestErrQueryClose(t *testing.T) {
t.Errorf("Query was ok or q.Error was nil")
}
}

func TestMaintenanceFunctions(t *testing.T) {
mapds := dstest.NewTestDatastore(true)
m := mount.New([]mount.Mount{
{Prefix: datastore.NewKey("/"), Datastore: mapds},
})

if err:= m.Check(); err.Error() != "checking datastore at /: test error" {
t.Errorf("Unexpected Check() error: %s", err)
}

if err:= m.CollectGarbage(); err.Error() != "gc on datastore at /: test error" {
t.Errorf("Unexpected CollectGarbage() error: %s", err)
}

if err:= m.Scrub(); err.Error() != "scrubbing datastore at /: test error" {
t.Errorf("Unexpected Scrub() error: %s", err)
}
}
15 changes: 14 additions & 1 deletion namespace/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
ds "github.com/ipfs/go-datastore"
ns "github.com/ipfs/go-datastore/namespace"
dsq "github.com/ipfs/go-datastore/query"
dstest "github.com/ipfs/go-datastore/test"
)

// Hook up gocheck into the "go test" runner.
Expand Down Expand Up @@ -79,7 +80,7 @@ func (ks *DSSuite) testBasic(c *C, prefix string) {
}

func (ks *DSSuite) TestQuery(c *C) {
mpds := ds.NewMapDatastore()
mpds := dstest.NewTestDatastore(true)
nsds := ns.Wrap(mpds, ds.NewKey("/foo"))

keys := strsToKeys([]string{
Expand Down Expand Up @@ -137,6 +138,18 @@ func (ks *DSSuite) TestQuery(c *C) {
expval, _ := expect[i].Value.([]byte)
c.Check(string(entval), Equals, string(expval))
}

if err := nsds.Datastore.(ds.CheckedDatastore).Check(); err != dstest.TestError {
c.Errorf("Unexpected Check() error: %s", err)
}

if err := nsds.Datastore.(ds.GCDatastore).CollectGarbage(); err != dstest.TestError {
c.Errorf("Unexpected CollectGarbage() error: %s", err)
}

if err := nsds.Datastore.(ds.ScrubbedDatastore).Scrub(); err != dstest.TestError {
c.Errorf("Unexpected Scrub() error: %s", err)
}
}

func strsToKeys(strs []string) []ds.Key {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@
"license": "MIT",
"name": "go-datastore",
"releaseCmd": "git commit -a -m \"gx publish $VERSION\"",
"version": "1.2.2"
"version": "1.3.0"
}

21 changes: 21 additions & 0 deletions sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,24 @@ func (b *syncBatch) Commit() error {
defer b.mds.Unlock()
return b.batch.Commit()
}

func (d *MutexDatastore) Check() error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit after the fact, but shouldn't these take the lock?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they should. Will push a fix in a bit

if c, ok := d.child.(ds.CheckedDatastore); ok {
return c.Check()
}
return nil
}

func (d *MutexDatastore) Scrub() error {
if c, ok := d.child.(ds.ScrubbedDatastore); ok {
return c.Scrub()
}
return nil
}

func (d *MutexDatastore) CollectGarbage() error {
if c, ok := d.child.(ds.GCDatastore); ok {
return c.CollectGarbage()
}
return nil
}
34 changes: 34 additions & 0 deletions syncmount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package syncmount

import (
"errors"
"fmt"
"io"
"sort"
"strings"
Expand Down Expand Up @@ -264,3 +265,36 @@ func (mt *mountBatch) Commit() error {
}
return nil
}

func (d *Datastore) Check() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(ds.CheckedDatastore); ok {
if err := c.Check(); err != nil {
return fmt.Errorf("checking datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}

func (d *Datastore) Scrub() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(ds.ScrubbedDatastore); ok {
if err := c.Scrub(); err != nil {
return fmt.Errorf("scrubbing datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}

func (d *Datastore) CollectGarbage() error {
for _, m := range d.mounts {
if c, ok := m.Datastore.(ds.GCDatastore); ok {
if err := c.CollectGarbage(); err != nil {
return fmt.Errorf("gc on datastore at %s: %s", m.Prefix.String(), err.Error())
}
}
}
return nil
}
20 changes: 20 additions & 0 deletions syncmount/mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/ipfs/go-datastore"
"github.com/ipfs/go-datastore/query"
mount "github.com/ipfs/go-datastore/syncmount"
dstest "github.com/ipfs/go-datastore/test"
)

func TestPutBadNothing(t *testing.T) {
Expand Down Expand Up @@ -371,3 +372,22 @@ func TestErrQueryClose(t *testing.T) {
t.Errorf("Query was ok or q.Error was nil")
}
}

func TestMaintenanceFunctions(t *testing.T) {
mapds := dstest.NewTestDatastore(true)
m := mount.New([]mount.Mount{
{Prefix: datastore.NewKey("/"), Datastore: mapds},
})

if err:= m.Check(); err.Error() != "checking datastore at /: test error" {
t.Errorf("Unexpected Check() error: %s", err)
}

if err:= m.CollectGarbage(); err.Error() != "gc on datastore at /: test error" {
t.Errorf("Unexpected CollectGarbage() error: %s", err)
}

if err:= m.Scrub(); err.Error() != "scrubbing datastore at /: test error" {
t.Errorf("Unexpected Scrub() error: %s", err)
}
}
Loading