Do not allow batch to be reused after commit (#67)

By explicitly not allowing the Batch to be reused, the Batch resources can be recycled, after Commit is called, for use in a new Batch.

In general, a Batch should not be reuesd for this or any other datastore becuase it is not guaranteed to be safely reusable for all datastores. Therefore, making this Batch explicitly not reusable allows better resource management as well as returning a clear error when the Batch is mistakenly reused.

Author: gammazero

datastore.go

@@ -2,6 +2,7 @@ package pebbleds
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"sync/atomic"
@@ -419,7 +420,12 @@ type Batch struct {
 
 var _ ds.Batch = (*Batch)(nil)
 
+var ErrBatchCommitted = errors.New("batch already committed and cannot be reused")
+
 func (b *Batch) Put(ctx context.Context, key ds.Key, value []byte) error {
+	if b.batch == nil {
+		return ErrBatchCommitted
+	}
 	err := b.batch.Set(key.Bytes(), value, b.writeOptions)
 	if err != nil {
 		return fmt.Errorf("pebble error during set within batch: %w", err)
@@ -428,6 +434,9 @@ func (b *Batch) Put(ctx context.Context, key ds.Key, value []byte) error {
 }
 
 func (b *Batch) Delete(ctx context.Context, key ds.Key) error {
+	if b.batch == nil {
+		return ErrBatchCommitted
+	}
 	err := b.batch.Delete(key.Bytes(), b.writeOptions)
 	if err != nil {
 		return fmt.Errorf("pebble error during delete within batch: %w", err)
@@ -436,6 +445,14 @@ func (b *Batch) Delete(ctx context.Context, key ds.Key) error {
 }
 
 func (b *Batch) Commit(ctx context.Context) error {
-	defer b.batch.Reset() // make batch reusable
-	return b.batch.Commit(b.writeOptions)
+	if b.batch == nil {
+		return ErrBatchCommitted
+	}
+	if err := b.batch.Commit(b.writeOptions); err != nil {
+		return err
+	}
+	// Recycle batch resources, when safe, for use in a new batch.
+	b.batch.Close()
+	b.batch = nil
+	return nil
 }

datastore_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/base32"
+	"errors"
 	"math/rand/v2"
 	"os"
 	"testing"
@@ -158,8 +159,13 @@ func TestBatch(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// second commit should do nothing, but should not cause an error
+	// second commit should return a ErrBatchCommitted error.
 	err = batch.Commit(ctx)
+	if !errors.Is(err, ErrBatchCommitted) {
+		t.Fatalf("expected error: %s, got %v", ErrBatchCommitted, err)
+	}
+
+	batch, err = ds.Batch(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}