ipfs / go-datastore

key-value datastore interfaces
MIT License
228 stars 64 forks source link

remove ThreadSafeDatastore #120

Closed Stebalien closed 5 years ago

Stebalien commented 5 years ago

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:

pub trait ThreadSafe {}

struct MyWrapper<D: Datastore> { ... }

impl<D: Datastore> ThreadSafe for MyWrapper<D> 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:

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.


Merge first:

hsanjuan commented 5 years ago

Heads up that this was a breaking API change and was released under a patch version bump (consider doing a minor release bump in that case, as most of us don't expect builds to break after go get -u=patch.

Stebalien commented 5 years ago

You're right, I should have done that. I'm too used to versions meaning nothing at this point...