From 4c211faa909c70ce90ea34cc6fbed663d62f1d67 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Mon, 2 Sep 2024 14:49:17 +1000 Subject: [PATCH] Revert "feat(op-program): Use `PebbleDB` for `DiskKV` (#11705)" (#11707) This reverts commit 8ab4d3d8e20f69464d9f5191c110b63f4d5a3df6. --- op-e2e/system_fpp_test.go | 3 -- op-program/host/host.go | 11 +--- op-program/host/kvstore/disk.go | 89 ++++++++++++++++++++------------ op-program/host/kvstore/kv.go | 3 -- op-program/host/kvstore/mem.go | 4 -- op-program/scripts/run-compat.sh | 2 +- 6 files changed, 60 insertions(+), 52 deletions(-) diff --git a/op-e2e/system_fpp_test.go b/op-e2e/system_fpp_test.go index 50b4b779f75e..c3598880b0cf 100644 --- a/op-e2e/system_fpp_test.go +++ b/op-e2e/system_fpp_test.go @@ -86,7 +86,6 @@ func applySpanBatchActivation(active bool, dp *genesis.DeployConfig) { // - update the state root via a tx // - run program func testVerifyL2OutputRootEmptyBlock(t *testing.T, detached bool, spanBatchActivated bool) { - t.Helper() InitParallel(t) ctx := context.Background() @@ -187,7 +186,6 @@ func testVerifyL2OutputRootEmptyBlock(t *testing.T, detached bool, spanBatchActi } func testVerifyL2OutputRoot(t *testing.T, detached bool, spanBatchActivated bool) { - t.Helper() InitParallel(t) ctx := context.Background() @@ -280,7 +278,6 @@ type FaultProofProgramTestScenario struct { // testFaultProofProgramScenario runs the fault proof program in several contexts, given a test scenario. func testFaultProofProgramScenario(t *testing.T, ctx context.Context, sys *System, s *FaultProofProgramTestScenario) { preimageDir := t.TempDir() - fppConfig := oppconf.NewConfig(sys.RollupConfig, sys.L2GenesisCfg.Config, s.L1Head, s.L2Head, s.L2OutputRoot, common.Hash(s.L2Claim), s.L2ClaimBlockNumber) fppConfig.L1URL = sys.NodeEndpoint("l1").RPC() fppConfig.L2URL = sys.NodeEndpoint("sequencer").RPC() diff --git a/op-program/host/host.go b/op-program/host/host.go index 582df9bf94af..ddffde7ee513 100644 --- a/op-program/host/host.go +++ b/op-program/host/host.go @@ -122,10 +122,6 @@ func FaultProofProgram(ctx context.Context, logger log.Logger, cfg *config.Confi func PreimageServer(ctx context.Context, logger log.Logger, cfg *config.Config, preimageChannel preimage.FileChannel, hintChannel preimage.FileChannel) error { var serverDone chan error var hinterDone chan error - logger.Info("Starting preimage server") - var kv kvstore.KV - - // Close the preimage/hint channels, and then kv store once the server and hinter have exited. defer func() { preimageChannel.Close() hintChannel.Close() @@ -137,12 +133,9 @@ func PreimageServer(ctx context.Context, logger log.Logger, cfg *config.Config, // Wait for hinter to complete <-hinterDone } - - if kv != nil { - kv.Close() - } }() - + logger.Info("Starting preimage server") + var kv kvstore.KV if cfg.DataDir == "" { logger.Info("Using in-memory storage") kv = kvstore.NewMemKV() diff --git a/op-program/host/kvstore/disk.go b/op-program/host/kvstore/disk.go index f2fe96402bbe..0f6bd19068ae 100644 --- a/op-program/host/kvstore/disk.go +++ b/op-program/host/kvstore/disk.go @@ -1,68 +1,93 @@ package kvstore import ( + "encoding/hex" "errors" "fmt" - "runtime" + "io" + "os" + "path" "sync" - "github.com/cockroachdb/pebble" "github.com/ethereum/go-ethereum/common" ) -// DiskKV is a disk-backed key-value store, with PebbleDB as the underlying DBMS. +// read/write mode for user/group/other, not executable. +const diskPermission = 0666 + +// DiskKV is a disk-backed key-value store, every key-value pair is a hex-encoded .txt file, with the value as content. // DiskKV is safe for concurrent use with a single DiskKV instance. +// DiskKV is safe for concurrent use between different DiskKV instances of the same disk directory as long as the +// file system supports atomic renames. type DiskKV struct { sync.RWMutex - db *pebble.DB + path string } // NewDiskKV creates a DiskKV that puts/gets pre-images as files in the given directory path. // The path must exist, or subsequent Put/Get calls will error when it does not. func NewDiskKV(path string) *DiskKV { - opts := &pebble.Options{ - Cache: pebble.NewCache(int64(32 * 1024 * 1024)), - MaxConcurrentCompactions: runtime.NumCPU, - Levels: []pebble.LevelOptions{ - {Compression: pebble.SnappyCompression}, - }, - } - db, err := pebble.Open(path, opts) - if err != nil { - panic(fmt.Errorf("failed to open pebbledb at %s: %w", path, err)) - } + return &DiskKV{path: path} +} - return &DiskKV{db: db} +func (d *DiskKV) pathKey(k common.Hash) string { + return path.Join(d.path, k.String()+".txt") } func (d *DiskKV) Put(k common.Hash, v []byte) error { d.Lock() defer d.Unlock() - return d.db.Set(k.Bytes(), v, pebble.NoSync) + f, err := openTempFile(d.path, k.String()+".txt.*") + if err != nil { + return fmt.Errorf("failed to open temp file for pre-image %s: %w", k, err) + } + defer os.Remove(f.Name()) // Clean up the temp file if it doesn't actually get moved into place + if _, err := f.Write([]byte(hex.EncodeToString(v))); err != nil { + _ = f.Close() + return fmt.Errorf("failed to write pre-image %s to disk: %w", k, err) + } + if err := f.Close(); err != nil { + return fmt.Errorf("failed to close temp pre-image %s file: %w", k, err) + } + + targetFile := d.pathKey(k) + if err := os.Rename(f.Name(), targetFile); err != nil { + return fmt.Errorf("failed to move temp dir %v to final destination %v: %w", f.Name(), targetFile, err) + } + return nil +} + +func openTempFile(dir string, nameTemplate string) (*os.File, error) { + f, err := os.CreateTemp(dir, nameTemplate) + // Directory has been deleted out from underneath us. Recreate it. + if errors.Is(err, os.ErrNotExist) { + if mkdirErr := os.MkdirAll(dir, 0777); mkdirErr != nil { + return nil, errors.Join(fmt.Errorf("failed to create directory %v: %w", dir, mkdirErr), err) + } + f, err = os.CreateTemp(dir, nameTemplate) + } + if err != nil { + return nil, err + } + return f, nil } func (d *DiskKV) Get(k common.Hash) ([]byte, error) { d.RLock() defer d.RUnlock() - - dat, closer, err := d.db.Get(k.Bytes()) + f, err := os.OpenFile(d.pathKey(k), os.O_RDONLY, diskPermission) if err != nil { - if errors.Is(err, pebble.ErrNotFound) { + if errors.Is(err, os.ErrNotExist) { return nil, ErrNotFound } - return nil, err + return nil, fmt.Errorf("failed to open pre-image file %s: %w", k, err) } - ret := make([]byte, len(dat)) - copy(ret, dat) - closer.Close() - return ret, nil -} - -func (d *DiskKV) Close() error { - d.Lock() - defer d.Unlock() - - return d.db.Close() + defer f.Close() // fine to ignore closing error here + dat, err := io.ReadAll(f) + if err != nil { + return nil, fmt.Errorf("failed to read pre-image from file %s: %w", k, err) + } + return hex.DecodeString(string(dat)) } var _ KV = (*DiskKV)(nil) diff --git a/op-program/host/kvstore/kv.go b/op-program/host/kvstore/kv.go index 054230950d0b..e75dcd699d9f 100644 --- a/op-program/host/kvstore/kv.go +++ b/op-program/host/kvstore/kv.go @@ -19,7 +19,4 @@ type KV interface { // It returns ErrNotFound when the pre-image cannot be found. // KV store implementations may return additional errors specific to the KV storage. Get(k common.Hash) ([]byte, error) - - // Closes the KV store. - Close() error } diff --git a/op-program/host/kvstore/mem.go b/op-program/host/kvstore/mem.go index 0c3b950b7e9c..9af540e235fa 100644 --- a/op-program/host/kvstore/mem.go +++ b/op-program/host/kvstore/mem.go @@ -37,7 +37,3 @@ func (m *MemKV) Get(k common.Hash) ([]byte, error) { } return slices.Clone(v), nil } - -func (m *MemKV) Close() error { - return nil -} diff --git a/op-program/scripts/run-compat.sh b/op-program/scripts/run-compat.sh index 296387ac5d0e..ed853a8d980b 100755 --- a/op-program/scripts/run-compat.sh +++ b/op-program/scripts/run-compat.sh @@ -5,7 +5,7 @@ SCRIPTS_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) COMPAT_DIR="${SCRIPTS_DIR}/../temp/compat" TESTNAME="${1?Must specify compat file to run}" -BASEURL="${2:-https://github.com/ethereum-optimism/chain-test-data/releases/download/2024-09-01}" +BASEURL="${2:-https://github.com/ethereum-optimism/chain-test-data/releases/download/2024-08-02}" URL="${BASEURL}/${TESTNAME}.tar.bz"