From 9f529bc350a59b53118f6dcd37c7fa1be861d923 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 15 Mar 2019 15:32:45 -0700 Subject: [PATCH] remove ThreadSafeDatastore It's a lie! We: 1. Assume that our datastores are thread-safe all over the place, not bothering to check for this interface. 2. Implement this interface for, e.g., the mount datastore that _may not_ be thread-safe (depending on the sub-datastores). Basically, there's no sane way to to do something like this in go. What we _want_ is: ```rust pub trait ThreadSafe {} struct MyWrapper { ... } impl ThreadSafe for MyWrapper where D: ThreadSafe {} ``` Actually, we don't even need this because rust has already done all the hard work with the `Sync` trait. .... But we're using go which barely has types. --- For completeness, it's actually possible to do this in go: ```go type threadSafeMixin struct{} func (threadSafeMixin) ThreadSafe() {} func NewWrapper(d Datastore) Datastore { if _, ok := d.(ThreadSafe) { return &struct{myWrapper, threadSafeMixin}{myWrapper{d}, threadSafeMixin{}} } return &myWrapper{d} } ``` Let's not. --- basic_ds.go | 4 +++- datastore.go | 8 -------- mount/mount.go | 2 -- sync/sync.go | 7 ++----- 4 files changed, 5 insertions(+), 16 deletions(-) diff --git a/basic_ds.go b/basic_ds.go index 5c5a0ad..ccfe14d 100644 --- a/basic_ds.go +++ b/basic_ds.go @@ -13,7 +13,9 @@ type MapDatastore struct { values map[Key][]byte } -// NewMapDatastore constructs a MapDatastore +// NewMapDatastore constructs a MapDatastore. It is _not_ thread-safe by +// default, wrap using sync.MutexWrap if you need thread safety (the answer here +// is usually yes). func NewMapDatastore() (d *MapDatastore) { return &MapDatastore{ values: make(map[Key][]byte), diff --git a/datastore.go b/datastore.go index b9ced85..eec1932 100644 --- a/datastore.go +++ b/datastore.go @@ -102,14 +102,6 @@ type Batching interface { // actually support batching. var ErrBatchUnsupported = errors.New("this datastore does not support batching") -// ThreadSafeDatastore is an interface that all threadsafe datastore should -// 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 { diff --git a/mount/mount.go b/mount/mount.go index 9e2622f..9d26f46 100644 --- a/mount/mount.go +++ b/mount/mount.go @@ -190,8 +190,6 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) { }), nil } -func (d *Datastore) IsThreadSafe() {} - func (d *Datastore) Close() error { for _, d := range d.mounts { err := d.Datastore.Close() diff --git a/sync/sync.go b/sync/sync.go index d222016..c9ec73a 100644 --- a/sync/sync.go +++ b/sync/sync.go @@ -15,8 +15,8 @@ type MutexDatastore struct { child ds.Datastore } -// MutexWrap constructs a datastore with a coarse lock around -// the entire datastore, for every single operation +// MutexWrap constructs a datastore with a coarse lock around the entire +// datastore, for every single operation. func MutexWrap(d ds.Datastore) *MutexDatastore { return &MutexDatastore{child: d} } @@ -26,9 +26,6 @@ func (d *MutexDatastore) Children() []ds.Datastore { return []ds.Datastore{d.child} } -// IsThreadSafe implements ThreadSafeDatastore -func (d *MutexDatastore) IsThreadSafe() {} - // Put implements Datastore.Put func (d *MutexDatastore) Put(key ds.Key, value []byte) (err error) { d.Lock()