-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
downloader: manual .lock remove may lead to race and creation of data files without .torrent #9782
Changes from all commits
68b0570
4fce504
f5cb658
316ee17
740edb3
5f79c83
b56f643
f70c580
8db030e
8b63f44
232b108
f84e75f
5317eca
36039f6
b9191ea
9efcaad
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 |
---|---|---|
|
@@ -57,9 +57,37 @@ func (tf *TorrentFiles) Create(name string, res []byte) error { | |
|
||
tf.lock.Lock() | ||
defer tf.lock.Unlock() | ||
return tf.create(filepath.Join(tf.dir, name), res) | ||
return tf.create(name, res) | ||
} | ||
func (tf *TorrentFiles) create(torrentFilePath string, res []byte) error { | ||
|
||
func (tf *TorrentFiles) CreateIfNotProhibited(name string, res []byte) (ts *torrent.TorrentSpec, prohibited, created bool, err error) { | ||
tf.lock.Lock() | ||
defer tf.lock.Unlock() | ||
prohibited, err = tf.newDownloadsAreProhibited(name) | ||
if err != nil { | ||
return nil, false, false, err | ||
} | ||
|
||
if !tf.exists(name) && !prohibited { | ||
err = tf.create(name, res) | ||
if err != nil { | ||
return nil, false, false, err | ||
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.
Does it makes sense to just check lock file exists on the start instead of checking it per file? Like, we started, downloads either prohibited or not, that's it. Could be changed only with restart. 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. “ instead of checking it per file? ” - this feature is just introduced by Giulio - to prevent downloading 1 type of file (he working on nee type of file): #9766 |
||
} | ||
} | ||
|
||
ts, err = tf.load(filepath.Join(tf.dir, name)) | ||
if err != nil { | ||
return nil, false, false, err | ||
} | ||
return ts, prohibited, false, nil | ||
} | ||
|
||
func (tf *TorrentFiles) create(name string, res []byte) error { | ||
if !strings.HasSuffix(name, ".torrent") { | ||
name += ".torrent" | ||
} | ||
torrentFilePath := filepath.Join(tf.dir, name) | ||
|
||
if len(res) == 0 { | ||
return fmt.Errorf("try to write 0 bytes to file: %s", torrentFilePath) | ||
} | ||
|
@@ -132,9 +160,13 @@ const ProhibitNewDownloadsFileName = "prohibit_new_downloads.lock" | |
// Erigon "download once" - means restart/upgrade/downgrade will not download files (and will be fast) | ||
// After "download once" - Erigon will produce and seed new files | ||
// Downloader will able: seed new files (already existing on FS), download uncomplete parts of existing files (if Verify found some bad parts) | ||
func (tf *TorrentFiles) prohibitNewDownloads(t string) error { | ||
func (tf *TorrentFiles) ProhibitNewDownloads(t string) error { | ||
tf.lock.Lock() | ||
defer tf.lock.Unlock() | ||
return tf.prohibitNewDownloads(t) | ||
} | ||
|
||
func (tf *TorrentFiles) prohibitNewDownloads(t string) error { | ||
// open or create file ProhibitNewDownloadsFileName | ||
f, err := os.OpenFile(filepath.Join(tf.dir, ProhibitNewDownloadsFileName), os.O_CREATE|os.O_RDONLY, 0644) | ||
if err != nil { | ||
|
@@ -174,9 +206,13 @@ func (tf *TorrentFiles) prohibitNewDownloads(t string) error { | |
return f.Sync() | ||
} | ||
|
||
func (tf *TorrentFiles) newDownloadsAreProhibited(name string) (bool, error) { | ||
func (tf *TorrentFiles) NewDownloadsAreProhibited(name string) (bool, error) { | ||
tf.lock.Lock() | ||
defer tf.lock.Unlock() | ||
return tf.newDownloadsAreProhibited(name) | ||
} | ||
|
||
func (tf *TorrentFiles) newDownloadsAreProhibited(name string) (bool, error) { | ||
f, err := os.OpenFile(filepath.Join(tf.dir, ProhibitNewDownloadsFileName), os.O_CREATE|os.O_APPEND|os.O_RDONLY, 0644) | ||
if err != nil { | ||
return false, err | ||
|
@@ -185,11 +221,11 @@ func (tf *TorrentFiles) newDownloadsAreProhibited(name string) (bool, error) { | |
var prohibitedList []string | ||
torrentListJsonBytes, err := io.ReadAll(f) | ||
if err != nil { | ||
return false, fmt.Errorf("newDownloadsAreProhibited: read file: %w", err) | ||
return false, fmt.Errorf("NewDownloadsAreProhibited: read file: %w", err) | ||
} | ||
if len(torrentListJsonBytes) > 0 { | ||
if err := json.Unmarshal(torrentListJsonBytes, &prohibitedList); err != nil { | ||
return false, fmt.Errorf("newDownloadsAreProhibited: unmarshal: %w", err) | ||
return false, fmt.Errorf("NewDownloadsAreProhibited: unmarshal: %w", err) | ||
} | ||
} | ||
for _, p := range prohibitedList { | ||
|
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.
prohibited
andcreated
results are not used anywhere - are they going to be used in a future PR? if not then should we remove them from the func signature?also looks like
created
is always returned as false - seems like we are missing a statement to set created totrue
and return that in the case of a file getting created