From 4ec4d6cd9cfc4e0109c264a582f4d308a927eed9 Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Mon, 12 May 2025 17:03:29 +1000 Subject: [PATCH] Fix broken file benchmark --- storage/file-piece.go | 26 +++++++++++++++++++---- storage/file.go | 2 +- storage/mark-complete_test.go | 7 +++++- storage/test/bench-piece-mark-complete.go | 2 +- torrent-piece-request-order.go | 3 +++ torrent.go | 17 +++++++++------ torrent_test.go | 2 +- 7 files changed, 44 insertions(+), 15 deletions(-) diff --git a/storage/file-piece.go b/storage/file-piece.go index c3968c5b..c8d2dce5 100644 --- a/storage/file-piece.go +++ b/storage/file-piece.go @@ -115,6 +115,10 @@ nextFile: _ = i //fmt.Printf("%v %#v %v\n", i, f, p) cmpl := me.t.getCompletion(p) + if cmpl.Err != nil { + err = fmt.Errorf("error getting completion for piece %d: %w", p, cmpl.Err) + return + } if !cmpl.Ok || !cmpl.Complete { continue nextFile } @@ -168,12 +172,26 @@ func (me *filePieceImpl) promotePartFile(f file) (err error) { return } +// Rename from if exists, and if so, to must not exist. +func (me *filePieceImpl) exclRenameIfExists(from, to string) (err error) { + _, err = os.Stat(from) + if errors.Is(err, fs.ErrNotExist) { + return nil + } + // We don't want anyone reading or writing to this until the rename completes. + f, err := os.OpenFile(to, os.O_CREATE|os.O_EXCL, 0) + if err != nil { + return fmt.Errorf("error creating destination file: %w", err) + } + f.Close() + return os.Rename(from, to) +} + func (me *filePieceImpl) onFileNotComplete(f file) (err error) { if me.partFiles() { - err = os.Rename(f.safeOsPath, f.partFilePath()) - // If we get ENOENT, the file may already be in the final location. - if err != nil && !errors.Is(err, fs.ErrNotExist) { - err = fmt.Errorf("renaming incomplete file: %w", err) + err = me.exclRenameIfExists(f.safeOsPath, f.partFilePath()) + if err != nil { + err = fmt.Errorf("restoring part file: %w", err) return } } diff --git a/storage/file.go b/storage/file.go index 3cf5f62b..c905be12 100644 --- a/storage/file.go +++ b/storage/file.go @@ -226,7 +226,7 @@ func (fst fileTorrentImplIO) readFileAt(file file, b []byte, off int64) (n int, if fst.fts.partFiles() { f, err = os.Open(file.partFilePath()) } - if errors.Is(err, fs.ErrNotExist) { + if err == nil && f == nil || errors.Is(err, fs.ErrNotExist) { f, err = os.Open(file.safeOsPath) } if errors.Is(err, fs.ErrNotExist) { diff --git a/storage/mark-complete_test.go b/storage/mark-complete_test.go index 7e50832d..482e9e0b 100644 --- a/storage/mark-complete_test.go +++ b/storage/mark-complete_test.go @@ -13,7 +13,12 @@ func BenchmarkMarkComplete(b *testing.B) { b, ci, test_storage.DefaultPieceSize, test_storage.DefaultNumPieces, 0) } b.Run("File", func(b *testing.B) { - ci := storage.NewFile(b.TempDir()) + ci := storage.NewFileOpts(storage.NewFileClientOpts{ + ClientBaseDir: b.TempDir(), + // TODO: Is the benchmark finding a bug? + //UsePartFiles: g.Some(false), + }) + //ci := storage.NewFile(b.TempDir()) b.Cleanup(func() { ci.Close() }) bench(b, ci) }) diff --git a/storage/test/bench-piece-mark-complete.go b/storage/test/bench-piece-mark-complete.go index dee46a88..8b3482fe 100644 --- a/storage/test/bench-piece-mark-complete.go +++ b/storage/test/bench-piece-mark-complete.go @@ -71,8 +71,8 @@ func BenchmarkPieceMarkComplete( qt.Assert(b, qt.Equals(pi.Completion(), storage.Completion{Complete: true, Ok: true})) n, err := pi.WriteTo(bytes.NewBuffer(readData[:0])) b.StopTimer() + qt.Check(b, qt.Equals(n, int64(len(data)))) qt.Assert(b, qt.IsNil(err)) - qt.Assert(b, qt.Equals(n, int64(len(data)))) qt.Assert(b, qt.IsTrue(bytes.Equal(readData[:n], data))) } } diff --git a/torrent-piece-request-order.go b/torrent-piece-request-order.go index eb4e00d7..7a9cf221 100644 --- a/torrent-piece-request-order.go +++ b/torrent-piece-request-order.go @@ -77,5 +77,8 @@ func (t *Torrent) addRequestOrderPiece(i int) { } func (t *Torrent) getPieceRequestOrder() *request_strategy.PieceRequestOrder { + if t.storage == nil { + return nil + } return t.cl.pieceRequestOrder[t.clientPieceRequestOrderKey()] } diff --git a/torrent.go b/torrent.go index e780c80f..c8775d7a 100644 --- a/torrent.go +++ b/torrent.go @@ -1535,13 +1535,16 @@ func (t *Torrent) logPieceRequestOrder() { return } pro := t.getPieceRequestOrder() - if pro != nil { - for item := range pro.Iter() { - t.slogger().Debug( - "piece request order item", "infohash", - item.Key.InfoHash, "piece", - item.Key.Index, "state", - item.State) + // This might require some optimization around Record to avoid performance issues when + // benchmarking. + if false { + if pro != nil { + for item := range pro.Iter() { + t.slogger().Debug("piece request order item", + "infohash", item.Key.InfoHash, + "piece", item.Key.Index, + "state", item.State) + } } } } diff --git a/torrent_test.go b/torrent_test.go index 7940fe2c..75df58fd 100644 --- a/torrent_test.go +++ b/torrent_test.go @@ -107,7 +107,7 @@ func BenchmarkUpdatePiecePriorities(b *testing.B) { t._completedPieces.Add(bitmap.BitIndex(i)) } t.DownloadPieces(0, t.numPieces()) - for i := 0; i < b.N; i += 1 { + for b.Loop() { t.updateAllPiecePriorities("") } } -- 2.51.0