refactor: car store trackers

carstore/error.go

@@ -0,0 +1,9 @@
+package carstore
+
+import "golang.org/x/xerrors"
+
+var ErrNotFound = xerrors.New("not found")
+
+func IsNotFound(err error) bool {
+	return xerrors.Is(err, ErrNotFound)
+}

carstore/read_only_blockstore.go

@@ -10,14 +10,14 @@ import (
 // CarReadOnlyStoreTracker tracks the lifecycle of a ReadOnly CAR Blockstore and makes it easy to create/get/cleanup the blockstores.
 // It's important to close a CAR Blockstore when done using it so that the backing CAR file can be closed.
 type CarReadOnlyStoreTracker struct {
-	mu     sync.Mutex
+	mu     sync.RWMutex
 	stores map[string]*blockstore.ReadOnly
 }
 
-func NewReadOnlyStoreTracker() (*CarReadOnlyStoreTracker, error) {
+func NewReadOnlyStoreTracker() *CarReadOnlyStoreTracker {
 	return &CarReadOnlyStoreTracker{
 		stores: make(map[string]*blockstore.ReadOnly),
-	}, nil
+	}
 }
 
 func (r *CarReadOnlyStoreTracker) Add(key string, bs *blockstore.ReadOnly) (bool, error) {
@@ -42,22 +42,22 @@ func (r *CarReadOnlyStoreTracker) GetOrCreate(key string, carFilePath string) (*
 
 	rdOnly, err := blockstore.OpenReadOnly(carFilePath, true)
 	if err != nil {
-		return nil, xerrors.Errorf("failed to open read-only blockstore, err=%w", err)
+		return nil, xerrors.Errorf("failed to open read-only blockstore: %w", err)
 	}
 	r.stores[key] = rdOnly
 
 	return rdOnly, nil
 }
 
 func (r *CarReadOnlyStoreTracker) Get(key string) (*blockstore.ReadOnly, error) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
+	r.mu.RLock()
+	defer r.mu.RUnlock()
 
 	if bs, ok := r.stores[key]; ok {
 		return bs, nil
 	}
 
-	return nil, xerrors.New("not found")
+	return nil, xerrors.Errorf("could not get blockstore for key %s: %w", ErrNotFound)
 }
 
 func (r *CarReadOnlyStoreTracker) CleanBlockStore(key string) error {
@@ -67,7 +67,7 @@ func (r *CarReadOnlyStoreTracker) CleanBlockStore(key string) error {
 	if bs, ok := r.stores[key]; ok {
 		delete(r.stores, key)
 		if err := bs.Close(); err != nil {
-			return xerrors.Errorf("failed to close read-only blockstore, err=%w", err)
+			return xerrors.Errorf("failed to close read-only blockstore: %w", err)
 		}
 	}
 

carstore/read_only_blockstore_test.go

@@ -0,0 +1,15 @@
+package carstore
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadOnlyStoreTracker(t *testing.T) {
+	k1 := "k1"
+	tracker := NewReadOnlyStoreTracker()
+
+	_, err := tracker.Get(k1)
+	require.True(t, IsNotFound(err))
+}

carstore/read_write_blockstore.go

@@ -11,14 +11,14 @@ import (
 // CarReadWriteStoreTracker tracks the lifecycle of a ReadWrite CAR Blockstore and makes it easy to create/get/cleanup the blockstores.
 // It's important to close a CAR Blockstore when done using it so that the backing CAR file can be closed.
 type CarReadWriteStoreTracker struct {
-	mu     sync.Mutex
+	mu     sync.RWMutex
 	stores map[string]*blockstore.ReadWrite
 }
 
-func NewCarReadWriteStoreTracker() (*CarReadWriteStoreTracker, error) {
+func NewCarReadWriteStoreTracker() *CarReadWriteStoreTracker {
 	return &CarReadWriteStoreTracker{
 		stores: make(map[string]*blockstore.ReadWrite),
-	}, nil
+	}
 }
 
 func (r *CarReadWriteStoreTracker) GetOrCreate(key string, carV2FilePath string, rootCid cid.Cid) (*blockstore.ReadWrite, error) {
@@ -31,22 +31,22 @@ func (r *CarReadWriteStoreTracker) GetOrCreate(key string, carV2FilePath string,
 
 	rwBs, err := blockstore.NewReadWrite(carV2FilePath, []cid.Cid{rootCid})
 	if err != nil {
-		return nil, xerrors.Errorf("failed to create read-write blockstore, err=%w", err)
+		return nil, xerrors.Errorf("failed to create read-write blockstore: %w", err)
 	}
 	r.stores[key] = rwBs
 
 	return rwBs, nil
 }
 
 func (r *CarReadWriteStoreTracker) Get(key string) (*blockstore.ReadWrite, error) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
+	r.mu.RLock()
+	defer r.mu.RUnlock()
 
 	if bs, ok := r.stores[key]; ok {
 		return bs, nil
 	}
 
-	return nil, xerrors.New("not found")
+	return nil, xerrors.Errorf("could not get blockstore for key %s: %w", ErrNotFound)
 }
 
 func (r *CarReadWriteStoreTracker) CleanBlockstore(key string) error {
@@ -58,7 +58,7 @@ func (r *CarReadWriteStoreTracker) CleanBlockstore(key string) error {
 
 		// calling a Finalize on a read-write blockstore is equivalent to closing it.
 		if err := bs.Finalize(); err != nil {
-			return xerrors.Errorf("finalize call failed, err=%w", err)
+			return xerrors.Errorf("finalize call failed: %w", err)
 		}
 	}
 

storagemarket/impl/client.go

@@ -81,11 +81,6 @@ func NewClient(
 	scn storagemarket.StorageClientNode,
 	options ...StorageClientOption,
 ) (*Client, error) {
-	st, err := carstore.NewReadOnlyStoreTracker()
-	if err != nil {
-		return nil, xerrors.Errorf("failed to create Read Only CAR Store tracker, err=%w", err)
-	}
-
 	c := &Client{
 		net:                     net,
 		dataTransfer:            dataTransfer,
@@ -94,7 +89,7 @@ func NewClient(
 		pubSub:                  pubsub.New(clientDispatcher),
 		readySub:                pubsub.New(shared.ReadyDispatcher),
 		pollingInterval:         DefaultPollingInterval,
-		readOnlyCARStoreTracker: st,
+		readOnlyCARStoreTracker: carstore.NewReadOnlyStoreTracker(),
 	}
 	storageMigrations, err := migrations.ClientMigrations.Build()
 	if err != nil {

storagemarket/impl/provider.go

@@ -111,11 +111,6 @@ func NewProvider(net network.StorageMarketNetwork,
 	storedAsk StoredAsk,
 	options ...StorageProviderOption,
 ) (storagemarket.StorageProvider, error) {
-	st, err := carstore.NewCarReadWriteStoreTracker()
-	if err != nil {
-		return nil, xerrors.Errorf("failed to create read write store tracker, err=%w", err)
-	}
-
 	h := &Provider{
 		net:                  net,
 		spn:                  spn,
@@ -127,7 +122,7 @@ func NewProvider(net network.StorageMarketNetwork,
 		dataTransfer:         dataTransfer,
 		pubSub:               pubsub.New(providerDispatcher),
 		readyMgr:             shared.NewReadyManager(),
-		readWriteBlockStores: st,
+		readWriteBlockStores: carstore.NewCarReadWriteStoreTracker(),
 	}
 	storageMigrations, err := migrations.ProviderMigrations.Build()
 	if err != nil {