Skip to content

Commit

Permalink
always close cache warm chan to prevent blocking (#14080)
Browse files Browse the repository at this point in the history
* always close cache warm chan to prevent blocking

* test that waitForCache does not block

* combine defers to reduce cognitive overhead

* lint

---------

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
  • Loading branch information
2 people authored and prestonvanloon committed Jun 5, 2024
1 parent b6f93e5 commit 26c0178
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 5 deletions.
12 changes: 7 additions & 5 deletions beacon-chain/db/filesystem/pruner.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,16 @@ func windowMin(latest, offset primitives.Slot) primitives.Slot {

func (p *blobPruner) warmCache() error {
p.Lock()
defer p.Unlock()
defer func() {
if !p.warmed {
p.warmed = true
close(p.cacheReady)
}
p.Unlock()
}()
if err := p.prune(0); err != nil {
return err
}
if !p.warmed {
p.warmed = true
close(p.cacheReady)
}
return nil
}

Expand Down
31 changes: 31 additions & 0 deletions beacon-chain/db/filesystem/pruner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ package filesystem

import (
"bytes"
"context"
"fmt"
"math"
"os"
"path"
"sort"
"testing"
"time"

"github.com/prysmaticlabs/prysm/v5/beacon-chain/verification"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v5/testing/require"
"github.com/prysmaticlabs/prysm/v5/testing/util"
"github.com/spf13/afero"
Expand All @@ -34,6 +37,34 @@ func TestTryPruneDir_CachedNotExpired(t *testing.T) {
require.Equal(t, 0, pruned)
}

func TestCacheWarmFail(t *testing.T) {
fs := afero.NewMemMapFs()
n := blobNamer{root: bytesutil.ToBytes32([]byte("derp")), index: 0}
bp := n.path()
mkdir := path.Dir(bp)
require.NoError(t, fs.MkdirAll(mkdir, directoryPermissions))

// Create an empty blob index in the fs by touching the file at a seemingly valid path.
fi, err := fs.Create(bp)
require.NoError(t, err)
require.NoError(t, fi.Close())

// Cache warm should fail due to the unexpected EOF.
pr, err := newBlobPruner(fs, 0)
require.NoError(t, err)
require.ErrorIs(t, pr.warmCache(), errPruningFailures)

// The cache warm has finished, so calling waitForCache with a super short deadline
// should not block or hit the context deadline.
ctx := context.Background()
ctx, cancel := context.WithDeadline(ctx, time.Now().Add(1*time.Millisecond))
defer cancel()
c, err := pr.waitForCache(ctx)
// We will get an error and a nil value for the cache if we hit the deadline.
require.NoError(t, err)
require.NotNil(t, c)
}

func TestTryPruneDir_CachedExpired(t *testing.T) {
t.Run("empty directory", func(t *testing.T) {
fs := afero.NewMemMapFs()
Expand Down

0 comments on commit 26c0178

Please sign in to comment.