From 3323116febbbbda9cf1a4e1fbccf48dce8ecfd74 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 12 Jul 2023 14:12:31 +0200 Subject: [PATCH 1/8] implement capped files --- capped_file.go | 191 ++++++++++++++++++++++++++++++++++++++++++++ capped_file_test.go | 134 +++++++++++++++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 capped_file.go create mode 100644 capped_file_test.go diff --git a/capped_file.go b/capped_file.go new file mode 100644 index 0000000..1d6d5ce --- /dev/null +++ b/capped_file.go @@ -0,0 +1,191 @@ +// bagdb: Simple datastorage +// Copyright 2021 billy authors +// SPDX-License-Identifier: BSD-3-Clause + +package billy + +import ( + "fmt" + "os" +) + +// cappedFile has an API-surface as if it were one file, but maps +// to a set of files on disk. These files are all capped in size +// to maxFileSize. +type cappedFile struct { + cap uint64 + files []*os.File +} + +// newCappedFile creates a cappedFile +func newCappedFile(basename string, nFiles int, cap uint64, readonly bool) (*cappedFile, error) { + flags := os.O_RDWR | os.O_CREATE + if readonly { + flags = os.O_RDONLY + } + var files []*os.File + for i := 0; i < nFiles; i++ { + var ( + f *os.File + err error + ) + if i == 0 { + f, err = os.OpenFile(basename, flags, 0666) + } else { + f, err = os.OpenFile(fmt.Sprintf("%v.cap.%d", basename, i), flags, 0666) + } + if err != nil { + // Clean-up: close opened files + for _, f := range files { + f.Close() + } + return nil, err + } + files = append(files, f) + } + return &cappedFile{ + cap: cap, + files: files, + }, nil +} + +// Write implements io.Writer +func (cf *cappedFile) Write(p []byte) (n int, err error) { + return cf.WriteAt(p, 0) +} + +// WriteAt writes len(b) bytes to the file starting at byte offset off. +// It returns the number of bytes written and an error, if any. +// WriteAt returns a non-nil error when n != len(b). +func (cf *cappedFile) WriteAt(data []byte, off int64) (n int, err error) { + var ( + offset = uint64(off) + written int // no of bytes read + totalLength = len(data) + fileNum = offset / cf.cap + ) + if fileNum >= uint64(len(cf.files)) { + return 0, ErrBadIndex + } + for ; written < totalLength; fileNum++ { + offset = offset % cf.cap + length := uint64(len(data)) + if offset+length > cf.cap { + // Write continuing in the next + length = cf.cap - offset + } + //fmt.Printf("WriteAt file-%d at %d <- b[:%d]\n", fileNum, offset, length) + if n, err := cf.files[fileNum].WriteAt(data[:length], int64(offset)); err != nil { + return written + n, err + } + data = data[length:] + written += int(length) + offset += length + } + return written, nil +} + +// Read implements io.Reader +func (cf *cappedFile) Read(p []byte) (n int, err error) { + return cf.ReadAt(p, 0) +} + +// ReadAt reads len(b) bytes from the file(s) starting at byte offset off. +// It returns the number of bytes read and the error, if any. +func (cf *cappedFile) ReadAt(b []byte, off int64) (n int, err error) { + var ( + offset = uint64(off) + read int // no of bytes read + fileNum = offset / cf.cap + ) + if fileNum >= uint64(len(cf.files)) { + return 0, ErrBadIndex + } + for ; read < len(b); fileNum++ { + offset = offset % cf.cap + length := uint64(len(b) - read) // no of bytes to read this iteration + if offset+length > cf.cap { + length = cf.cap - offset + } + //fmt.Printf("ReadAt file-%d at %d -> b[%d:%d]\n", fileNum, offset, read, length) + if n, err := cf.files[fileNum].ReadAt(b[read:uint64(read)+length], int64(offset)); err != nil { + return read + n, err + } + read += int(length) + offset += length + } + return read, nil +} + +// Sync calls *os.File Sync on the backing-files. +func (cf *cappedFile) Sync() error { + var err error + for _, f := range cf.files { + if e := f.Sync(); e != nil && err == nil { + err = e + } + } + return err +} + +// Close closes all files. +func (cf *cappedFile) Close() error { + var err error + for _, f := range cf.files { + if e := f.Close(); e != nil && err == nil { + err = e + } + } + return err +} + +// Truncate changes the size of the file. +func (cf *cappedFile) Truncate(size int64) error { + var err error + for i, f := range cf.files { + // Files below the truncation limit are ignored. + if uint64(i+1)*cf.cap <= uint64(size) { + continue + } + // Files fully above the truncation limit are truncated to zero. + if uint64(i)*cf.cap > uint64(size) { + if e := f.Truncate(0); e != nil && err == nil { + err = e + } + continue + } + fSize := size % int64(cf.cap) + //fmt.Printf("Truncate file-%d to %d (total size %d)\n", i, fSize, size) + if e := f.Truncate(fSize); e != nil && err == nil { + err = e + } + } + return err +} + +type Sizer interface { + Size() int64 // length in bytes for regular files; system-dependent for others +} +type sizeDummy struct { + size int64 +} + +func (s *sizeDummy) Size() int64 { + return s.size +} + +func (cf *cappedFile) Stat() (Sizer, error) { + var size int64 + var err error + for _, f := range cf.files { + finfo, e := f.Stat() + if e != nil && err != nil { + err = e + } + if finfo != nil { + size += finfo.Size() + } + } + return &sizeDummy{size}, err + +} diff --git a/capped_file_test.go b/capped_file_test.go new file mode 100644 index 0000000..65a1468 --- /dev/null +++ b/capped_file_test.go @@ -0,0 +1,134 @@ +// bagdb: Simple datastorage +// Copyright 2021 billy authors +// SPDX-License-Identifier: BSD-3-Clause + +package billy + +import ( + "bytes" + "os" + "testing" +) + +// wipe deletes the files. +func wipe(t *testing.T, cf *cappedFile) { + t.Helper() + var err error + for _, f := range cf.files { + if e := os.RemoveAll(f.Name()); e != nil && err == nil { + err = e + } + } + if err != nil { + t.Fatal(err) + } +} + +func diskSize(t *testing.T, cf *cappedFile) int { + t.Helper() + var size int + for _, f := range cf.files { + finfo, err := f.Stat() + if err != nil { + t.Fatal(err) + } + size += int(finfo.Size()) + //t.Logf("file %d size %d total %d", j, finfo.Size(), have) + } + return size +} + +func TestWriteAt(t *testing.T) { + // Max filesize: 50 bytes. + // 10 individual files. + // Mox storage capacity: 500 bytes. + f, err := newCappedFile("multifile-test", 10, 50, false) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { wipe(t, f) }) + + { + // Writing data at offset 55 means that it should write into + // the second file, five bytes in. + // [0,0,0,0,0,m,o,o,h] + want := []byte("mooh") + if _, err := f.WriteAt(want, 55); err != nil { + t.Fatal(err) + } + have := make([]byte, 4) + if _, err := f.ReadAt(have, 55); err != nil { + t.Fatal(err) + } + if !bytes.Equal(have, want) { + t.Fatalf("have %x want %x", have, want) + } + } + + { + // Test data that starts at one file, fully saturates a second, and + // ends a bit into the third. + want := []byte("miao01234567890123456789012345678901234567890123456789woof") + if _, err := f.WriteAt(want, 96); err != nil { + t.Fatal(err) + } + have := make([]byte, 58) + if _, err := f.ReadAt(have, 96); err != nil { + t.Fatal(err) + } + if !bytes.Equal(have, want) { + t.Fatalf("have %x want %x", have, want) + } + } +} + +func TestTruncate(t *testing.T) { + // Max filesize: 50 bytes. + // 10 individual files. + // Mox storage capacity: 500 bytes. + f, err := newCappedFile("multifile-test", 10, 50, false) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { wipe(t, f) }) + // Fill with data + if _, err := f.WriteAt(make([]byte, 470), 20); err != nil { + t.Fatal(err) + } + // The total size of all files should == 490 + if have, want := diskSize(t, f), 490; have != want { + t.Fatalf("have %d want %d", have, want) + } + for i := 480; i > 0; i -= 10 { + if err := f.Truncate(int64(i)); err != nil { + t.Fatal(err) + } + // The total size of all files should == i + if have, want := diskSize(t, f), i; have != want { + t.Fatalf("have %d want %d", have, want) + } + } +} + +func TestReadonly(t *testing.T) { + // Create in readonly -- should fail + f, err := newCappedFile("multifile-ro-test", 10, 50, true) + if err == nil { + t.Fatal("want error trying to create files in readonly, got none") + } + // Create in RW, should be ok + f, err = newCappedFile("multifile-ro-test", 10, 50, false) + if err != nil { + t.Fatal(err) + } + f.Close() + // The files now exist, so should be ok to open + f, err = newCappedFile("multifile-ro-test", 10, 50, true) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { wipe(t, f) }) + if _, err := f.WriteAt([]byte("mooh"), 55); err == nil { + t.Fatalf("want error trying to write files in readonly, got none") + } +} From 188cebe285b33f7e340a92a287b89b95caf34207 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 13 Jul 2023 13:33:57 +0200 Subject: [PATCH 2/8] shelf: use capped files --- capped_file.go | 31 ++++++------------------------- shelf.go | 10 +++++----- 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/capped_file.go b/capped_file.go index 1d6d5ce..c61bd6c 100644 --- a/capped_file.go +++ b/capped_file.go @@ -49,11 +49,6 @@ func newCappedFile(basename string, nFiles int, cap uint64, readonly bool) (*cap }, nil } -// Write implements io.Writer -func (cf *cappedFile) Write(p []byte) (n int, err error) { - return cf.WriteAt(p, 0) -} - // WriteAt writes len(b) bytes to the file starting at byte offset off. // It returns the number of bytes written and an error, if any. // WriteAt returns a non-nil error when n != len(b). @@ -85,11 +80,6 @@ func (cf *cappedFile) WriteAt(data []byte, off int64) (n int, err error) { return written, nil } -// Read implements io.Reader -func (cf *cappedFile) Read(p []byte) (n int, err error) { - return cf.ReadAt(p, 0) -} - // ReadAt reads len(b) bytes from the file(s) starting at byte offset off. // It returns the number of bytes read and the error, if any. func (cf *cappedFile) ReadAt(b []byte, off int64) (n int, err error) { @@ -143,8 +133,11 @@ func (cf *cappedFile) Close() error { func (cf *cappedFile) Truncate(size int64) error { var err error for i, f := range cf.files { - // Files below the truncation limit are ignored. + // Files below the truncation limit are expanded to the limit. if uint64(i+1)*cf.cap <= uint64(size) { + if e := f.Truncate(int64(cf.cap)); e != nil && err == nil { + err = e + } continue } // Files fully above the truncation limit are truncated to zero. @@ -163,18 +156,7 @@ func (cf *cappedFile) Truncate(size int64) error { return err } -type Sizer interface { - Size() int64 // length in bytes for regular files; system-dependent for others -} -type sizeDummy struct { - size int64 -} - -func (s *sizeDummy) Size() int64 { - return s.size -} - -func (cf *cappedFile) Stat() (Sizer, error) { +func (cf *cappedFile) Stat() (os.FileInfo, error) { var size int64 var err error for _, f := range cf.files { @@ -186,6 +168,5 @@ func (cf *cappedFile) Stat() (Sizer, error) { size += finfo.Size() } } - return &sizeDummy{size}, err - + return &fileinfoMock{size: size}, nil } diff --git a/shelf.go b/shelf.go index 3a88c66..1c46bf3 100644 --- a/shelf.go +++ b/shelf.go @@ -83,17 +83,17 @@ func openShelf(path string, slotSize uint32, onData onShelfDataFn, readonly bool fileSize int h = shelfHeader{Magic, curVersion, slotSize} fname = fmt.Sprintf("bkt_%08d.bag", slotSize) - flags = os.O_RDWR | os.O_CREATE ) - if readonly { - flags = os.O_RDONLY - } var ( f store err error ) if path != "" { - f, err = os.OpenFile(filepath.Join(path, fname), flags, 0666) + // Max 5 files @ 2GB each. + // We also want to ensure that the cap is a multiple of the slot size, so no + // slot crosses a file boundary. + cap := uint64(2*1024*1024 - 2*1024*1024%slotSize) + f, err = newCappedFile(filepath.Join(path, fname), 5, cap, readonly) if err != nil { return nil, err } From 9125881f4fdfc0e17d1e959eb577c6d17158b61a Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 15 Jul 2023 20:10:26 +0200 Subject: [PATCH 3/8] capped_file: increase test coverage, fix out-of-bounds check --- capped_file.go | 8 ++++++-- capped_file_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/capped_file.go b/capped_file.go index c61bd6c..0154ac2 100644 --- a/capped_file.go +++ b/capped_file.go @@ -59,7 +59,9 @@ func (cf *cappedFile) WriteAt(data []byte, off int64) (n int, err error) { totalLength = len(data) fileNum = offset / cf.cap ) - if fileNum >= uint64(len(cf.files)) { + // Check if the write starts or ends out of bounds + if fileNum >= uint64(len(cf.files)) || + (offset+uint64(len(data)))/cf.cap >= uint64(len(cf.files)) { return 0, ErrBadIndex } for ; written < totalLength; fileNum++ { @@ -88,7 +90,9 @@ func (cf *cappedFile) ReadAt(b []byte, off int64) (n int, err error) { read int // no of bytes read fileNum = offset / cf.cap ) - if fileNum >= uint64(len(cf.files)) { + // Check if the read starts or ends out of bounds + if fileNum >= uint64(len(cf.files)) || + (offset+uint64(len(b)))/cf.cap >= uint64(len(cf.files)) { return 0, ErrBadIndex } for ; read < len(b); fileNum++ { diff --git a/capped_file_test.go b/capped_file_test.go index 65a1468..d593543 100644 --- a/capped_file_test.go +++ b/capped_file_test.go @@ -6,6 +6,7 @@ package billy import ( "bytes" + "errors" "os" "testing" ) @@ -132,3 +133,31 @@ func TestReadonly(t *testing.T) { t.Fatalf("want error trying to write files in readonly, got none") } } + +func TestOutOfBounds(t *testing.T) { + f, err := newCappedFile("multifile-ro-test", 10, 50, false) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + f.Close() + wipe(t, f) + }) + // Read starting OOB + if _, err := f.ReadAt(make([]byte, 10), 501); !errors.Is(err, ErrBadIndex) { + t.Fatalf("want %v have %v", ErrBadIndex, err) + } + // Read reaching into OOB + if _, err := f.ReadAt(make([]byte, 10), 495); !errors.Is(err, ErrBadIndex) { + t.Fatalf("want %v have %v", ErrBadIndex, err) + } + // Write starting OOB + if _, err := f.WriteAt(make([]byte, 10), 501); !errors.Is(err, ErrBadIndex) { + t.Fatalf("want %v have %v", ErrBadIndex, err) + } + // Write reaching into OOB + if _, err := f.WriteAt(make([]byte, 10), 495); !errors.Is(err, ErrBadIndex) { + t.Fatalf("want %v have %v", ErrBadIndex, err) + } + +} From d3362e920eb713fa90e85b52b75f2e7a55984161 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 18 Jul 2023 19:57:48 +0200 Subject: [PATCH 4/8] inc tests --- capped_file_test.go | 10 ++++++++++ shelf.go | 6 ++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/capped_file_test.go b/capped_file_test.go index d593543..fe3e7e8 100644 --- a/capped_file_test.go +++ b/capped_file_test.go @@ -109,6 +109,16 @@ func TestTruncate(t *testing.T) { t.Fatalf("have %d want %d", have, want) } } + // And "truncate" back up again + for i := 0; i < 480; i += 10 { + if err := f.Truncate(int64(i)); err != nil { + t.Fatal(err) + } + // The total size of all files should == i + if have, want := diskSize(t, f), i; have != want { + t.Fatalf("have %d want %d", have, want) + } + } } func TestReadonly(t *testing.T) { diff --git a/shelf.go b/shelf.go index 1c46bf3..92f2e07 100644 --- a/shelf.go +++ b/shelf.go @@ -83,10 +83,8 @@ func openShelf(path string, slotSize uint32, onData onShelfDataFn, readonly bool fileSize int h = shelfHeader{Magic, curVersion, slotSize} fname = fmt.Sprintf("bkt_%08d.bag", slotSize) - ) - var ( - f store - err error + f store + err error ) if path != "" { // Max 5 files @ 2GB each. From cd91ed6cc67b8386abba0b58dd7351f9e7d41483 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 19 Jul 2023 10:00:35 +0200 Subject: [PATCH 5/8] capped_file: change how file layout works --- capped_file.go | 69 ++++++++++++++++--------------------------- capped_file_test.go | 27 ++++++++++++----- cmd/billyfuzz/main.go | 21 ++++++------- db.go | 18 +++++++++-- shelf.go | 13 ++++---- shelf_test.go | 26 ++++++++-------- 6 files changed, 91 insertions(+), 83 deletions(-) diff --git a/capped_file.go b/capped_file.go index 0154ac2..cfbf84d 100644 --- a/capped_file.go +++ b/capped_file.go @@ -23,6 +23,9 @@ func newCappedFile(basename string, nFiles int, cap uint64, readonly bool) (*cap if readonly { flags = os.O_RDONLY } + if cap == 0 { + nFiles = 1 + } var files []*os.File for i := 0; i < nFiles; i++ { var ( @@ -52,63 +55,40 @@ func newCappedFile(basename string, nFiles int, cap uint64, readonly bool) (*cap // WriteAt writes len(b) bytes to the file starting at byte offset off. // It returns the number of bytes written and an error, if any. // WriteAt returns a non-nil error when n != len(b). +// +// Internally, a write will only touch one file, and may cause the cap to be exceeeded. func (cf *cappedFile) WriteAt(data []byte, off int64) (n int, err error) { var ( - offset = uint64(off) - written int // no of bytes read - totalLength = len(data) - fileNum = offset / cf.cap + fNum = uint64(0) + fOffset = off ) - // Check if the write starts or ends out of bounds - if fileNum >= uint64(len(cf.files)) || - (offset+uint64(len(data)))/cf.cap >= uint64(len(cf.files)) { - return 0, ErrBadIndex + if cf.cap > 0 { + fNum = uint64(off) / cf.cap + fOffset = off % int64(cf.cap) } - for ; written < totalLength; fileNum++ { - offset = offset % cf.cap - length := uint64(len(data)) - if offset+length > cf.cap { - // Write continuing in the next - length = cf.cap - offset - } - //fmt.Printf("WriteAt file-%d at %d <- b[:%d]\n", fileNum, offset, length) - if n, err := cf.files[fileNum].WriteAt(data[:length], int64(offset)); err != nil { - return written + n, err - } - data = data[length:] - written += int(length) - offset += length + // Check if the write is out of bounds + if fNum >= uint64(len(cf.files)) { + return 0, ErrBadIndex } - return written, nil + return cf.files[fNum].WriteAt(data, fOffset) } // ReadAt reads len(b) bytes from the file(s) starting at byte offset off. // It returns the number of bytes read and the error, if any. func (cf *cappedFile) ReadAt(b []byte, off int64) (n int, err error) { var ( - offset = uint64(off) - read int // no of bytes read - fileNum = offset / cf.cap + fNum = uint64(0) + fOffset = off ) - // Check if the read starts or ends out of bounds - if fileNum >= uint64(len(cf.files)) || - (offset+uint64(len(b)))/cf.cap >= uint64(len(cf.files)) { - return 0, ErrBadIndex + if cf.cap > 0 { + fNum = uint64(off) / cf.cap + fOffset = off % int64(cf.cap) } - for ; read < len(b); fileNum++ { - offset = offset % cf.cap - length := uint64(len(b) - read) // no of bytes to read this iteration - if offset+length > cf.cap { - length = cf.cap - offset - } - //fmt.Printf("ReadAt file-%d at %d -> b[%d:%d]\n", fileNum, offset, read, length) - if n, err := cf.files[fileNum].ReadAt(b[read:uint64(read)+length], int64(offset)); err != nil { - return read + n, err - } - read += int(length) - offset += length + // Check if the read is out of bounds + if fNum >= uint64(len(cf.files)) { + return 0, ErrBadIndex } - return read, nil + return cf.files[fNum].ReadAt(b, fOffset) } // Sync calls *os.File Sync on the backing-files. @@ -135,6 +115,9 @@ func (cf *cappedFile) Close() error { // Truncate changes the size of the file. func (cf *cappedFile) Truncate(size int64) error { + if cf.cap == 0 { + return cf.files[0].Truncate(size) + } var err error for i, f := range cf.files { // Files below the truncation limit are expanded to the limit. diff --git a/capped_file_test.go b/capped_file_test.go index fe3e7e8..904ffea 100644 --- a/capped_file_test.go +++ b/capped_file_test.go @@ -7,6 +7,7 @@ package billy import ( "bytes" "errors" + "io" "os" "testing" ) @@ -67,8 +68,7 @@ func TestWriteAt(t *testing.T) { } { - // Test data that starts at one file, fully saturates a second, and - // ends a bit into the third. + // Test data that starts at one file, and severely exceeds the cap. want := []byte("miao01234567890123456789012345678901234567890123456789woof") if _, err := f.WriteAt(want, 96); err != nil { t.Fatal(err) @@ -80,6 +80,20 @@ func TestWriteAt(t *testing.T) { if !bytes.Equal(have, want) { t.Fatalf("have %x want %x", have, want) } + // We can now do a 'small write' into the second file, which should not + // 'disturb' the data in the first file. This is an implementation + // quirk, not a "desired feature", but still should be checked by tests. + if _, err := f.WriteAt(make([]byte, 50), 100); err != nil { + t.Fatal(err) + } + // Check original data in first file + have = make([]byte, 58) + if _, err := f.ReadAt(have, 96); err != nil { + t.Fatal(err) + } + if !bytes.Equal(have, want) { + t.Fatalf("have %x want %x", have, want) + } } } @@ -158,16 +172,15 @@ func TestOutOfBounds(t *testing.T) { t.Fatalf("want %v have %v", ErrBadIndex, err) } // Read reaching into OOB - if _, err := f.ReadAt(make([]byte, 10), 495); !errors.Is(err, ErrBadIndex) { + if _, err := f.ReadAt(make([]byte, 10), 495); !errors.Is(err, io.EOF) { t.Fatalf("want %v have %v", ErrBadIndex, err) } // Write starting OOB if _, err := f.WriteAt(make([]byte, 10), 501); !errors.Is(err, ErrBadIndex) { t.Fatalf("want %v have %v", ErrBadIndex, err) } - // Write reaching into OOB - if _, err := f.WriteAt(make([]byte, 10), 495); !errors.Is(err, ErrBadIndex) { - t.Fatalf("want %v have %v", ErrBadIndex, err) + // Write exceeding the global cap (not enforced) + if _, err := f.WriteAt(make([]byte, 10), 495); err != nil { + t.Fatal(err) } - } diff --git a/cmd/billyfuzz/main.go b/cmd/billyfuzz/main.go index 4a524a5..6dbb7f9 100644 --- a/cmd/billyfuzz/main.go +++ b/cmd/billyfuzz/main.go @@ -3,7 +3,6 @@ package main import ( crand "crypto/rand" "crypto/sha256" - "encoding/hex" "fmt" "math/rand" "os" @@ -68,18 +67,17 @@ func doOpenDb(ctx *cli.Context, onData billy.OnDataFn) (billy.Database, error) { func doFuzz(ctx *cli.Context) error { var ( - hasher = sha256.New() hashes = make(map[uint64]string) onData = func(key uint64, size uint32, data []byte) { if verbose { fmt.Printf("init key %x val %x\n", key, data[:20]) } - hasher.Reset() - hashes[key] = hex.EncodeToString(hasher.Sum(data)) + hashes[key] = fmt.Sprintf("%x", sha256.Sum256(data)) } db, err = doOpenDb(ctx, onData) ) if err != nil { + fmt.Printf("Error opening db: %v\n", err) return err } var ( @@ -109,8 +107,7 @@ func doFuzz(ctx *cli.Context) error { l := int(min) + rand.Intn(int(max-min)) data := make([]byte, l) _, _ = crand.Read(data) - hasher.Reset() - sum := hex.EncodeToString(hasher.Sum(data)) + sum := fmt.Sprintf("%x", sha256.Sum256(data)) key, err := db.Put(data) if err != nil { panic(err) @@ -126,19 +123,23 @@ func doFuzz(ctx *cli.Context) error { for key, want = range hashes { break } - //fmt.Printf("Checking %d bytes data at key %d\n", len(want), key) data, err := db.Get(key) if err != nil { + fmt.Printf("Checking data at key %d\n", key) panic(err) } // check the data - hasher.Reset() - have := hex.EncodeToString(hasher.Sum(data)) + have := fmt.Sprintf("%x", sha256.Sum256(data)) if have != want { - panic(fmt.Sprintf("key %v\nhave %v\n, want %v\n", key, have, want)) + fmt.Printf("key %v\nhave %d bytes, hash %v\n, want %v\n", key, + len(data), have, want) + panic("GET failure") } case 2: // DELETE var key uint64 + if len(hashes) == 0 { + continue + } for key = range hashes { break } diff --git a/db.go b/db.go index 8c8f867..668b5cb 100644 --- a/db.go +++ b/db.go @@ -86,9 +86,20 @@ type database struct { } type Options struct { - Path string + // Path is the path to where the billy-files are stored. An empty value + // means 'memory-mode', in which nothing is stored to disk. + Path string + // MaxFileSize is the maximum size of files used. A value of zero means that + // there is no maximum. + MaxFileSize uint64 + // ShelfFilesCount is the amount of files to open for each shelf. The total + // storage capacity of any shelf is thus `MaxFileSize * ShelfFilesCount`. + // If `MaxFileSize==0`, then this value is not used. + ShelfFilesCount int + // Readonly means that the db cannot be used for writing. Readonly bool - Snappy bool // unused for now + // Snappy is not used + Snappy bool } // Open opens a (new or existing) database, with configurable limits. The given @@ -113,7 +124,8 @@ func Open(opts Options, slotSizeFn SlotSizeFn, onData OnDataFn) (Database, error return nil, fmt.Errorf("slot sizes must be in increasing order") } prevSlotSize = slotSize - shelf, err := openShelf(opts.Path, slotSize, wrapShelfDataFn(len(db.shelves), slotSize, onData), opts.Readonly) + shelf, err := openShelf(opts.Path, slotSize, wrapShelfDataFn(len(db.shelves), slotSize, onData), + opts.MaxFileSize, opts.ShelfFilesCount, opts.Readonly) if err != nil { db.Close() // Close shelves return nil, err diff --git a/shelf.go b/shelf.go index 92f2e07..0e21e01 100644 --- a/shelf.go +++ b/shelf.go @@ -68,10 +68,13 @@ type shelfHeader struct { // If the shelf already exists, it's opened and read, which populates the // internal gap-list. // The onData callback is optional, and can be nil. -func openShelf(path string, slotSize uint32, onData onShelfDataFn, readonly bool) (*shelf, error) { +func openShelf(path string, slotSize uint32, onData onShelfDataFn, maxFileSize uint64, nFiles int, readonly bool) (*shelf, error) { if slotSize < minSlotSize { return nil, fmt.Errorf("slot size %d smaller than minimum (%d)", slotSize, minSlotSize) } + if maxFileSize != 0 && nFiles == 0 { + return nil, fmt.Errorf("number of files (%d) must be non-zero if max file size (%d) set", nFiles, maxFileSize) + } if path != "" { // empty path == in-memory database if finfo, err := os.Stat(path); err != nil { return nil, err @@ -87,11 +90,7 @@ func openShelf(path string, slotSize uint32, onData onShelfDataFn, readonly bool err error ) if path != "" { - // Max 5 files @ 2GB each. - // We also want to ensure that the cap is a multiple of the slot size, so no - // slot crosses a file boundary. - cap := uint64(2*1024*1024 - 2*1024*1024%slotSize) - f, err = newCappedFile(filepath.Join(path, fname), 5, cap, readonly) + f, err = newCappedFile(filepath.Join(path, fname), nFiles, maxFileSize, readonly) if err != nil { return nil, err } @@ -284,7 +283,7 @@ func (s *shelf) Get(slot uint64) ([]byte, error) { } data, err := s.readSlot(make([]byte, s.slotSize), slot) if err != nil { - return nil, fmt.Errorf("%w: %v", ErrBadIndex, err) + return nil, fmt.Errorf("%w: slot %d, slotsize %d, %v", ErrBadIndex, slot, s.slotSize, err) } return data, nil } diff --git a/shelf_test.go b/shelf_test.go index 08fa13b..e529bbb 100644 --- a/shelf_test.go +++ b/shelf_test.go @@ -42,11 +42,11 @@ func TestBasicsOnDisk(t *testing.T) { testBasics(t, t.TempDir()) } func testBasics(t *testing.T, path string) { { // Pre-instance failures // can't open non-existing directory - if _, err := openShelf("/baz/bonk/foobar/gazonk", 10, nil, false); err == nil { + if _, err := openShelf("/baz/bonk/foobar/gazonk", 10, nil, 0, 0, false); err == nil { t.Fatal("expected error") } // Can't point path to a file - if _, err := openShelf("./README.md", 10, nil, false); err == nil { + if _, err := openShelf("./README.md", 10, nil, 0, 0, false); err == nil { t.Fatal("expected error") } } @@ -226,7 +226,7 @@ func checkIdentical(fileA, fileB string) error { func setup(t *testing.T, path string) (*shelf, func()) { t.Helper() - a, err := openShelf(path, 200, nil, false) + a, err := openShelf(path, 200, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -379,12 +379,12 @@ func TestCompaction(t *testing.T) { haveOnData = append(haveOnData, data[0]) } /// Now open them as shelves - a, err = openShelf(pA, 10, onData, false) + a, err = openShelf(pA, 10, onData, 0, 0, false) if err != nil { t.Fatal(err) } a.Close() - b, err = openShelf(pB, 10, nil, false) + b, err = openShelf(pB, 10, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -438,7 +438,7 @@ func TestCompaction2(t *testing.T) { p := t.TempDir() /// Now open them as shelves openAndStore := func(data string) { - a, err := openShelf(p, 10, nil, false) + a, err := openShelf(p, 10, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -453,14 +453,14 @@ func TestCompaction2(t *testing.T) { var data []byte _, err := openShelf(p, 10, func(slot uint64, x []byte) { data = append(data, x...) - }, false) + }, 0, 0, false) if err != nil { t.Fatal(err) } return string(data) } openAndDel := func(deletes ...int) { - a, err := openShelf(p, 10, nil, false) + a, err := openShelf(p, 10, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -496,7 +496,7 @@ func TestCompaction2(t *testing.T) { func TestShelfRO(t *testing.T) { p := t.TempDir() - a, err := openShelf(p, 20, nil, false) + a, err := openShelf(p, 20, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -526,7 +526,7 @@ func TestShelfRO(t *testing.T) { out := new(strings.Builder) a, err = openShelf(p, 20, func(slot uint64, data []byte) { fmt.Fprintf(out, "%d:%d, ", slot, len(data)) - }, true) + }, 0, 0, true) if err != nil { t.Fatal(err) } @@ -554,7 +554,7 @@ func TestShelfRO(t *testing.T) { out = new(strings.Builder) a, err = openShelf(p, 20, func(slot uint64, data []byte) { fmt.Fprintf(out, "%d:%d, ", slot, len(data)) - }, false) + }, 0, 0, false) if err != nil { t.Fatal(err) } @@ -570,7 +570,7 @@ func TestShelfRO(t *testing.T) { func TestDelete(t *testing.T) { p := t.TempDir() - a, err := openShelf(p, 20, nil, false) + a, err := openShelf(p, 20, nil, 0, 0, false) if err != nil { t.Fatal(err) } @@ -642,7 +642,7 @@ func TestVersion(t *testing.T) { if err := os.WriteFile(filepath.Join(p, fname), tc.hdr, 0o777); err != nil { t.Fatal(err) } - _, err := openShelf(p, size, nil, false) + _, err := openShelf(p, size, nil, 0, 0, false) if err == nil { t.Fatal("expected error") } From dd5e58e75b433eb84454fcf7d2da316b9ec5f243 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 19 Jul 2023 10:59:03 +0200 Subject: [PATCH 6/8] minor --- shelf_test.go | 4 ++++ capped_file.go => store_capped.go | 0 capped_file_test.go => store_capped_test.go | 0 3 files changed, 4 insertions(+) rename capped_file.go => store_capped.go (100%) rename capped_file_test.go => store_capped_test.go (100%) diff --git a/shelf_test.go b/shelf_test.go index e529bbb..0983099 100644 --- a/shelf_test.go +++ b/shelf_test.go @@ -49,6 +49,10 @@ func testBasics(t *testing.T, path string) { if _, err := openShelf("./README.md", 10, nil, 0, 0, false); err == nil { t.Fatal("expected error") } + // Can't provide nonzero maxfilesize but zero files + if _, err := openShelf("foo", 10, nil, 1, 0, false); err == nil { + t.Fatal("expected error") + } } b, cleanup := setup(t, path) defer cleanup() diff --git a/capped_file.go b/store_capped.go similarity index 100% rename from capped_file.go rename to store_capped.go diff --git a/capped_file_test.go b/store_capped_test.go similarity index 100% rename from capped_file_test.go rename to store_capped_test.go From 7a9da949edd5185aeec39a2b1b6f922945d37b83 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 20 Jul 2023 09:17:10 +0200 Subject: [PATCH 7/8] store_capped; two subtle flaws --- shelf_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ store_capped.go | 18 ++++++++++++++---- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/shelf_test.go b/shelf_test.go index 0983099..2eef387 100644 --- a/shelf_test.go +++ b/shelf_test.go @@ -655,3 +655,48 @@ func TestVersion(t *testing.T) { } } } + +func TestCappedTruncate(t *testing.T) { + p := t.TempDir() + + // Max file size 100 + // Max files 5, total capacity 100 + // SHelf size 27 + 4 = 31 + s, err := openShelf(p, 27+itemHeaderSize, nil, 100, 5, false) + if err != nil { + t.Fatal(err) + } + // Fill up the capped files and delete an item to trigger a truncation + var keys []uint64 + for i := byte(1); i <= 10; i++ { + if key, err := s.Put(bytes.Repeat([]byte{i}, 27)); err != nil { + t.Fatalf("failed to put item %d: %v", i, err) + } else { + keys = append(keys, key) + } + } + err = s.Delete(keys[len(keys)-1]) + if err != nil { + t.Fatal(err) + } + keys = keys[:len(keys)-1] + fmt.Printf("There are %d elements\n", len(keys)) + // Reopen the shelf to compact it + s.Close() + s, err = openShelf(p, 27+itemHeaderSize, nil, 100, 5, false) + if err != nil { + t.Fatal(err) + } + // Verify that all values survived compactions + for i, key := range keys { + val, err := s.Get(key) + if err != nil { + t.Errorf("failed to retrieve slot %d: %v", i, err) + continue + } + want := bytes.Repeat([]byte{byte(i + 1)}, 27) + if !bytes.Equal(val, want) { + t.Errorf("item %d mismatch: have %x, want %x", i, val, want) + } + } +} diff --git a/store_capped.go b/store_capped.go index cfbf84d..a96450f 100644 --- a/store_capped.go +++ b/store_capped.go @@ -70,6 +70,7 @@ func (cf *cappedFile) WriteAt(data []byte, off int64) (n int, err error) { if fNum >= uint64(len(cf.files)) { return 0, ErrBadIndex } + //fmt.Printf("file-%d, write %d bytes @ %d (total offset %d)\n", fNum, len(data), fOffset, off) return cf.files[fNum].WriteAt(data, fOffset) } @@ -122,9 +123,9 @@ func (cf *cappedFile) Truncate(size int64) error { for i, f := range cf.files { // Files below the truncation limit are expanded to the limit. if uint64(i+1)*cf.cap <= uint64(size) { - if e := f.Truncate(int64(cf.cap)); e != nil && err == nil { - err = e - } + //if e := f.Truncate(int64(cf.cap)); e != nil && err == nil { + // err = e + //} continue } // Files fully above the truncation limit are truncated to zero. @@ -152,7 +153,16 @@ func (cf *cappedFile) Stat() (os.FileInfo, error) { err = e } if finfo != nil { - size += finfo.Size() + // If a file exceeds the cap, then the next file will contain a + // corresponding empty-data section in the beginning. Therefore, + // we must not count that twice. Easiest to just count to the cap. + // This is a bit hacky, and would be more correct if we also + // ensure that the 'next' file is non-empty. + a := finfo.Size() + if a > int64(cf.cap) { + a = int64(cf.cap) + } + size += a } } return &fileinfoMock{size: size}, nil From a920bfe7fc0b5a0d59964bcc210765033af56fdf Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 20 Jul 2023 09:40:34 +0200 Subject: [PATCH 8/8] better fix --- db.go | 14 ++++----- shelf_test.go | 1 - store_capped.go | 72 +++++++++++++++++++++++--------------------- store_capped_test.go | 25 ++++++++------- 4 files changed, 58 insertions(+), 54 deletions(-) diff --git a/db.go b/db.go index 668b5cb..15d453c 100644 --- a/db.go +++ b/db.go @@ -89,13 +89,13 @@ type Options struct { // Path is the path to where the billy-files are stored. An empty value // means 'memory-mode', in which nothing is stored to disk. Path string - // MaxFileSize is the maximum size of files used. A value of zero means that + // ShelfFileSize is the maximum size of files used. A value of zero means that // there is no maximum. - MaxFileSize uint64 - // ShelfFilesCount is the amount of files to open for each shelf. The total - // storage capacity of any shelf is thus `MaxFileSize * ShelfFilesCount`. - // If `MaxFileSize==0`, then this value is not used. - ShelfFilesCount int + ShelfFileSize uint64 + // ShelfFileCount is the amount of files to open for each shelf. The total + // storage capacity of any shelf is thus `ShelfFileSize * ShelfFileCount`. + // If `ShelfFileSize==0`, then this value is not used. + ShelfFileCount int // Readonly means that the db cannot be used for writing. Readonly bool // Snappy is not used @@ -125,7 +125,7 @@ func Open(opts Options, slotSizeFn SlotSizeFn, onData OnDataFn) (Database, error } prevSlotSize = slotSize shelf, err := openShelf(opts.Path, slotSize, wrapShelfDataFn(len(db.shelves), slotSize, onData), - opts.MaxFileSize, opts.ShelfFilesCount, opts.Readonly) + opts.ShelfFileSize, opts.ShelfFileCount, opts.Readonly) if err != nil { db.Close() // Close shelves return nil, err diff --git a/shelf_test.go b/shelf_test.go index 2eef387..427e339 100644 --- a/shelf_test.go +++ b/shelf_test.go @@ -680,7 +680,6 @@ func TestCappedTruncate(t *testing.T) { t.Fatal(err) } keys = keys[:len(keys)-1] - fmt.Printf("There are %d elements\n", len(keys)) // Reopen the shelf to compact it s.Close() s, err = openShelf(p, 27+itemHeaderSize, nil, 100, 5, false) diff --git a/store_capped.go b/store_capped.go index a96450f..a3d2d2b 100644 --- a/store_capped.go +++ b/store_capped.go @@ -119,51 +119,53 @@ func (cf *cappedFile) Truncate(size int64) error { if cf.cap == 0 { return cf.files[0].Truncate(size) } - var err error - for i, f := range cf.files { - // Files below the truncation limit are expanded to the limit. - if uint64(i+1)*cf.cap <= uint64(size) { - //if e := f.Truncate(int64(cf.cap)); e != nil && err == nil { - // err = e - //} - continue - } - // Files fully above the truncation limit are truncated to zero. - if uint64(i)*cf.cap > uint64(size) { - if e := f.Truncate(0); e != nil && err == nil { - err = e - } - continue - } - fSize := size % int64(cf.cap) - //fmt.Printf("Truncate file-%d to %d (total size %d)\n", i, fSize, size) - if e := f.Truncate(fSize); e != nil && err == nil { - err = e + // Files fully below the truncation limit are left in place. This is subtly + // wrong: the os.File Stat() operation _expands_ a file if it is too small, + // so ideally we should maybe "truncate up" the files in passing. + // However, it's possible that the files have exceeded the capcacity, + // and we must not truncate them. + + i := int(uint64(size) / cf.cap) + //fmt.Printf("Truncate file-%d to %d (total size %d)\n", i, size%int64(cf.cap), size) + if err := cf.files[i].Truncate(size % int64(cf.cap)); err != nil { + return err + } + // Files fully above the truncation limit are truncated to zero. + for i++; i < len(cf.files); i++ { + //fmt.Printf("Truncate file-%d to 0\n", i) + if err := cf.files[i].Truncate(0); err != nil { + return err } } - return err + return nil } func (cf *cappedFile) Stat() (os.FileInfo, error) { var size int64 var err error + var uncounted int64 for _, f := range cf.files { finfo, e := f.Stat() - if e != nil && err != nil { - err = e - } - if finfo != nil { - // If a file exceeds the cap, then the next file will contain a - // corresponding empty-data section in the beginning. Therefore, - // we must not count that twice. Easiest to just count to the cap. - // This is a bit hacky, and would be more correct if we also - // ensure that the 'next' file is non-empty. - a := finfo.Size() - if a > int64(cf.cap) { - a = int64(cf.cap) + if e != nil { + if err != nil { + err = e } - size += a + continue + } + s := finfo.Size() + if s == 0 { + size += uncounted + break + } + if cf.cap == 0 || s < int64(cf.cap) { + // File is not at capacity. No need to continue. + size += s + break + } else { + // File is at or over capacity. Add cf.cap bytes, and remember the overflow + size += int64(cf.cap) + uncounted = s - int64(cf.cap) } } - return &fileinfoMock{size: size}, nil + return &fileinfoMock{size: size}, err } diff --git a/store_capped_test.go b/store_capped_test.go index 904ffea..be391ec 100644 --- a/store_capped_test.go +++ b/store_capped_test.go @@ -107,8 +107,10 @@ func TestTruncate(t *testing.T) { } t.Cleanup(func() { wipe(t, f) }) // Fill with data - if _, err := f.WriteAt(make([]byte, 470), 20); err != nil { - t.Fatal(err) + for i := 0; i < 47; i++ { + if _, err := f.WriteAt(make([]byte, 10), int64(20+i*10)); err != nil { + t.Fatal(err) + } } // The total size of all files should == 490 if have, want := diskSize(t, f), 490; have != want { @@ -124,15 +126,16 @@ func TestTruncate(t *testing.T) { } } // And "truncate" back up again - for i := 0; i < 480; i += 10 { - if err := f.Truncate(int64(i)); err != nil { - t.Fatal(err) - } - // The total size of all files should == i - if have, want := diskSize(t, f), i; have != want { - t.Fatalf("have %d want %d", have, want) - } - } + // NO longer supported + //for i := 0; i < 480; i += 10 { + // if err := f.Truncate(int64(i)); err != nil { + // t.Fatal(err) + // } + // // The total size of all files should == i + // if have, want := diskSize(t, f), i; have != want { + // t.Fatalf("have %d want %d", have, want) + // } + //} } func TestReadonly(t *testing.T) {