Skip to content

Commit 7539376

Browse files
committed
Fix deadlock with AutoShrink and Close, pass tests. Bump v1.8.1
1 parent 4b73f5d commit 7539376

File tree

5 files changed

+90
-42
lines changed

5 files changed

+90
-42
lines changed

Diff for: README.md

+43-9
Original file line numberDiff line numberDiff line change
@@ -125,18 +125,52 @@ func main() {
125125
<details><summary>Using Multiple Typed Maps Together</summary>
126126

127127
```go
128-
store := persist.New()
129-
defer store.Close()
128+
package main
129+
130+
import (
131+
"log"
132+
"time"
133+
"github.com/Jipok/go-persist"
134+
)
135+
136+
type User struct {
137+
Name string
138+
Age int
139+
}
140+
141+
type Product struct {
142+
Name string
143+
Price float64
144+
}
130145

131-
users, _ := persist.Map[User](store, "users")
132-
products, _ := persist.Map[Product](store, "products")
133-
sessions, _ := persist.Map[Session](store, "sessions")
146+
type Session struct {
147+
UserID string
148+
Expiration int64
149+
}
150+
151+
func main() {
152+
store := persist.New()
153+
defer store.Close()
154+
155+
// Create typed maps for different entity types
156+
users, _ := persist.Map[User](store, "users")
157+
products, _ := persist.Map[Product](store, "products")
158+
sessions, _ := persist.Map[Session](store, "sessions")
134159

135-
store.Open("app.db")
160+
// Create or load store file
161+
err := store.Open("app.db")
162+
if err != nil {
163+
log.Fatal(err)
164+
}
165+
166+
// Set up automatic compaction
167+
store.StartAutoShrink(time.Minute, 1.8)
136168

137-
users.Set("user1", User{Name: "Alice", Age: 30})
138-
products.Set("product42", Product{Name: "Gadget", Price: 49.99})
139-
sessions.SetAsync("sess12345", Session{UserID: "user1", Expire: 1718557123})
169+
// Use each map independently
170+
users.Set("u1", User{Name: "Admin", Age: 35})
171+
products.Set("p1", Product{Name: "Widget", Price: 19.99})
172+
sessions.SetAsync("sess123", Session{UserID: "u1", Expiration: 1718557123})
173+
}
140174
```
141175
</details>
142176

Diff for: benchmark-load/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.24.1
55
replace github.com/Jipok/go-persist => ../
66

77
require (
8-
github.com/Jipok/go-persist v1.7.0
8+
github.com/Jipok/go-persist v1.8.0
99
github.com/goccy/go-json v0.10.5
1010
github.com/tidwall/buntdb v1.3.2
1111
go.etcd.io/bbolt v1.4.0

Diff for: benchmark/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module bench
33
go 1.23.6
44

55
require (
6-
github.com/Jipok/go-persist v1.7.0
6+
github.com/Jipok/go-persist v1.8.0
77
github.com/goccy/go-json v0.10.5
88
github.com/tidwall/buntdb v1.3.2
99
go.etcd.io/bbolt v1.4.0

Diff for: wal.go

+19-11
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const WalHeader = "go-persist 1"
2525
const DefaultSyncInterval = time.Second
2626

2727
var (
28-
// ErrKeyNotFound is returned when the key is not found in the storage
2928
ErrKeyNotFound = errors.New("key not found")
3029
ErrNotLoaded = errors.New("store is not loaded")
3130
ErrShrinkInProgress = errors.New("shrink operation is already in progress")
@@ -198,6 +197,7 @@ func (s *Store) processRecords() error {
198197
recordsChan := make(chan recordData, 100)
199198

200199
// Start a goroutine for reading the records concurrently
200+
var outErr error
201201
go func() {
202202
defer close(recordsChan)
203203
for {
@@ -206,7 +206,7 @@ func (s *Store) processRecords() error {
206206
if err == io.EOF {
207207
break
208208
}
209-
log.Println("go-persist: error reading record:", err)
209+
outErr = errors.New("error reading record: " + err.Error())
210210
break
211211
}
212212
s.totalWALRecords.Add(1)
@@ -226,7 +226,7 @@ func (s *Store) processRecords() error {
226226
// Registered map found - process the record via its interface
227227
pm, _ := mapVal.(persistMapI)
228228
if err := pm.processRecord(rec.op, rec.fullKey[idx+1:], rec.valueStr); err != nil {
229-
log.Println("go-persist: failed processing record for key", rec.fullKey, "error:", err)
229+
return errors.New("go-persist: failed processing record for key `" + rec.fullKey + "`:" + err.Error())
230230
}
231231
} else {
232232
// No matching map – save the raw record as a string in orphanRecords
@@ -239,6 +239,10 @@ func (s *Store) processRecords() error {
239239
}
240240
}
241241

242+
if outErr != nil {
243+
return outErr
244+
}
245+
242246
return nil
243247
}
244248

@@ -254,7 +258,6 @@ func (s *Store) Close() error {
254258
// Stop auto-shrink if enabled
255259
if s.stopAutoShrink != nil {
256260
close(s.stopAutoShrink)
257-
s.stopAutoShrink = nil
258261
}
259262

260263
// Signal background FSyncAll to stop and wait for it to finish
@@ -481,17 +484,22 @@ func (s *Store) Shrink() error {
481484
defer s.wg.Done()
482485
s.mu.Unlock()
483486

487+
stopShrinking := func() {
488+
s.mu.Lock()
489+
s.shrinking = false
490+
s.mu.Unlock()
491+
}
492+
484493
// Create temporary file for the compacted WAL
485494
tmpPath := s.path + ".tmp"
486495
tmpFile, err := os.Create(tmpPath)
487496
if err != nil {
488-
s.shrinking = false
489497
return err
490498
}
491499

492500
// Write the WAL header
493501
if _, err := tmpFile.WriteString(WalHeader + "\n"); err != nil {
494-
s.shrinking = false
502+
stopShrinking()
495503
return err
496504
}
497505

@@ -527,7 +535,7 @@ func (s *Store) Shrink() error {
527535
return true
528536
})
529537
if outErr != nil {
530-
s.shrinking = false
538+
stopShrinking()
531539
return outErr
532540
}
533541

@@ -544,13 +552,13 @@ func (s *Store) Shrink() error {
544552
return true
545553
})
546554
if outErr != nil {
547-
s.shrinking = false
555+
stopShrinking()
548556
return outErr
549557
}
550558

551559
// Sync file to disk before obtaining lock to minimize lock duration
552560
if err := tmpFile.Sync(); err != nil {
553-
s.shrinking = false
561+
stopShrinking()
554562
return err
555563
}
556564

@@ -571,13 +579,13 @@ func (s *Store) Shrink() error {
571579
// Write the locally copied pending records outside the lock
572580
for _, rec := range localPending {
573581
if _, err := tmpFile.WriteString(rec); err != nil {
574-
s.shrinking = false
582+
stopShrinking()
575583
return err
576584
}
577585
recordCounter++
578586
}
579587
if err := tmpFile.Sync(); err != nil {
580-
s.shrinking = false
588+
stopShrinking()
581589
return err
582590
}
583591
}

Diff for: wal_test.go

+26-20
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"os"
66
"strconv"
7+
"strings"
78
"sync"
89
"testing"
910
)
@@ -365,27 +366,32 @@ func TestStore_InvalidRecordMidFile(t *testing.T) {
365366
// Open the store
366367
store := New()
367368
err = store.Open(path)
368-
if err != nil {
369-
t.Fatalf("failed to open store: %v", err)
369+
if !strings.Contains(err.Error(), "invalid record header format") {
370+
t.Fatalf("expected: invalid record header format. Got: %v", err)
370371
}
371372
defer store.Close()
372373

373-
// Attempt to retrieve key "first" which was written before the invalid record.
374-
// Expect to get the value 100.
375-
firstVal, err := Get[int](store, "first")
376-
if err != nil {
377-
t.Errorf("failed to get key 'first': %v", err)
378-
}
379-
if firstVal != 100 {
380-
t.Errorf("expected key 'first' to have value 100, got %d", firstVal)
381-
}
382-
383-
// Attempt to retrieve key "second" which was written after the invalid record.
384-
// Since the invalid record stops further processing, key "second" should not be found.
385-
secondVal, err := Get[int](store, "second")
386-
if err == nil {
387-
t.Errorf("expected error for key 'second' due to invalid record mid file, got value %d", secondVal)
388-
} else if !errors.Is(err, ErrKeyNotFound) {
389-
t.Errorf("expected ErrKeyNotFound for key 'second', got: %v", err)
390-
}
374+
_, err = Get[int](store, "first")
375+
if !strings.Contains(err.Error(), "store is not loaded") {
376+
t.Fatalf("expected: store is not loaded. Got: %v", err)
377+
}
378+
379+
// // Attempt to retrieve key "first" which was written before the invalid record.
380+
// // Expect to get the value 100.
381+
// firstVal, err := Get[int](store, "first")
382+
// if err != nil {
383+
// t.Errorf("failed to get key 'first': %v", err)
384+
// }
385+
// if firstVal != 100 {
386+
// t.Errorf("expected key 'first' to have value 100, got %d", firstVal)
387+
// }
388+
389+
// // Attempt to retrieve key "second" which was written after the invalid record.
390+
// // Since the invalid record stops further processing, key "second" should not be found.
391+
// secondVal, err := Get[int](store, "second")
392+
// if err == nil {
393+
// t.Errorf("expected error for key 'second' due to invalid record mid file, got value %d", secondVal)
394+
// } else if !errors.Is(err, ErrKeyNotFound) {
395+
// t.Errorf("expected ErrKeyNotFound for key 'second', got: %v", err)
396+
// }
391397
}

0 commit comments

Comments
 (0)