-
-
Notifications
You must be signed in to change notification settings - Fork 633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do t.storage.Flush() on torrent completion, and on storage.Close() #755
Changes from 2 commits
03714df
82f028f
78741df
b22e5c0
f6b0733
842f561
32d3086
259201f
320081c
2acf848
8af4a58
6e9f2d0
d091a27
df962eb
eaa4a6e
0c79727
9e69d94
cf9f013
a79295e
e454c69
ce57a48
a4a87d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,11 +19,30 @@ func (ms *MMapSpan) Append(mMap mmap.MMap) { | |
ms.mMaps = append(ms.mMaps, mMap) | ||
} | ||
|
||
func (ms *MMapSpan) Flush() (errs []error) { | ||
ms.mu.Lock() | ||
defer ms.mu.Unlock() | ||
for _, mMap := range ms.mMaps { | ||
err := mMap.Flush() | ||
if err != nil { | ||
errs = append(errs, err) | ||
} | ||
} | ||
// This is for issue 211. | ||
ms.mMaps = nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems strange, why woudl Flush do this, especially if htere is no error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, mistake. removed. |
||
ms.InitIndex() | ||
return | ||
} | ||
|
||
func (ms *MMapSpan) Close() (errs []error) { | ||
ms.mu.Lock() | ||
defer ms.mu.Unlock() | ||
for _, mMap := range ms.mMaps { | ||
err := mMap.Unmap() | ||
err := mMap.Flush() | ||
if err != nil { | ||
errs = append(errs, err) | ||
} | ||
err = mMap.Unmap() | ||
if err != nil { | ||
errs = append(errs, err) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2461,7 +2461,11 @@ func (t *Torrent) pieceRequestIndexOffset(piece pieceIndex) RequestIndex { | |
} | ||
|
||
func (t *Torrent) updateComplete() { | ||
before := t.Complete.Bool() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there might be a more suitable place to put this, there should be a method that's only called when torrent completion transitions to complete from incomplete. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, I don't see such method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see the confusion. This flushes the entire torrent, when the entire torrent is complete. Do you want perhaps flush the individual memory-maps, when associated pieces become complete? That would be a more gradual approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for not spotting this before. Similarly, if this was the approach, the Flush would move on to the piece implementation. Let me know your thoughts. |
||
t.Complete.SetBool(t.haveAllPieces()) | ||
if !before && t.Complete.Bool() { | ||
_ = t.storage.Flush() | ||
} | ||
} | ||
|
||
func (t *Torrent) cancelRequest(r RequestIndex) *Peer { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's common enough not to have a Flush method, that teh caller can check for Flush != nil in the TorrentImpl struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done