Skip to content

Commit

Permalink
Fix: deadlock on sync all
Browse files Browse the repository at this point in the history
  • Loading branch information
yunginnanet committed Jul 3, 2024
1 parent 2b89223 commit 1158810
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 18 deletions.
20 changes: 13 additions & 7 deletions bitcask/bitcask.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,23 @@ func (db *DB) Meta() models.Metadata {
return m
}

func (db *DB) allStores() map[string]database.Filer {
var stores = make(map[string]database.Filer)
for n, s := range db.store {
stores[n] = s
}
return stores
}

// AllStores returns a map of the names of all bitcask datastores and the corresponding Filers.
func (db *DB) AllStores() map[string]database.Filer {
if err := db.init(); err != nil {
panic(err)
}
db.mu.RLock()
defer db.mu.RUnlock()
var stores = make(map[string]database.Filer)
for n, s := range db.store {
stores[n] = s
}
return stores
ast := db.allStores()
db.mu.RUnlock()
return ast
}

func (db *DB) _init() error {
Expand Down Expand Up @@ -504,7 +509,7 @@ func (db *DB) CloseAll() error {
return err
}
func (db *DB) addAllStoresToMeta() {
storeMap := db.AllStores()
storeMap := db.allStores()
storeNames := make([]string, len(storeMap))
for name := range storeMap {
storeNames = append(storeNames, name)
Expand All @@ -513,6 +518,7 @@ func (db *DB) addAllStoresToMeta() {
}

// SyncAll syncs all pogreb datastores.
// TODO: investigate locking here, right now if we try to hold a lock during a backup we'll hang :^)
func (db *DB) SyncAll() error {
db.addAllStoresToMeta()
var errs = make([]error, 0)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ require (
)

retract (
v0.4.3 // deadlock
v0.4.0 // broken metadata system
v0.3.0 // doesn't pass go vet
)
33 changes: 22 additions & 11 deletions pogreb/pogreb.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,25 +175,37 @@ func (db *DB) Meta() models.Metadata {
return m
}

func (db *DB) UpdateMetrics() {
db.mu.RLock()
defer db.mu.RUnlock()
func (db *DB) updateMetrics() {
for _, s := range db.store {
s.metrics = s.DB.Metrics()
if s != nil && s.DB != nil {
s.metrics = s.DB.Metrics()
}
}
}

// AllStores returns a map of the names of all pogreb datastores and the corresponding Filers.
func (db *DB) AllStores() map[string]database.Filer {
func (db *DB) UpdateMetrics() {
db.mu.RLock()
defer db.mu.RUnlock()
db.updateMetrics()
db.mu.RUnlock()
}

func (db *DB) allStores() map[string]database.Filer {
var stores = make(map[string]database.Filer)
for n, s := range db.store {
stores[n] = s
}
return stores
}

// AllStores returns a map of the names of all pogreb datastores and the corresponding Filers.
func (db *DB) AllStores() map[string]database.Filer {

db.mu.RLock()
ast := db.allStores()
db.mu.RUnlock()
return ast
}

// FIXME: not returning the error is probably pretty irresponsible.

// OpenDB will either open an existing set of pogreb datastores at the given directory, or it will create a new one.
Expand Down Expand Up @@ -522,21 +534,19 @@ func (db *DB) CloseAll() error {
}

func (db *DB) allMetrics() map[string]*pogreb.Metrics {
db.UpdateMetrics()
db.updateMetrics()
allmet := make(map[string]*pogreb.Metrics, len(db.store))
db.mu.RLock()
for name, store := range db.store {
if store == nil || store.Backend().(*pogreb.DB) == nil {
continue
}
allmet[name] = store.metrics
}
db.mu.RUnlock()
return allmet
}

func (db *DB) addAllStoresToMeta() {
storeMap := db.AllStores()
storeMap := db.allStores()
storeNames := make([]string, len(storeMap))
for name := range storeMap {
storeNames = append(storeNames, name)
Expand All @@ -550,6 +560,7 @@ func (db *DB) syncMetaValues() {
}

// SyncAll syncs all pogreb datastores.
// TODO: investigate locking here, right now if we try to hold a lock during a backup we'll hang :^)
func (db *DB) SyncAll() error {
db.syncMetaValues()
var errs = make([]error, 0)
Expand Down

0 comments on commit 1158810

Please sign in to comment.