Fri 09 Sep 2005 08:07:05 AM UTC, comment #28:
Here is even more improved version.
As additional to mentioned 1,2,3 it also has
4. preventing exception on canceling bittorent with torrent file being somehow removed (i had plenty of them)
|
Fri 09 Sep 2005 02:54:11 AM UTC, comment #27:
Here is improoved version of the patch.
1. Now removing swarmer in file_completed is also purged.
2. When file has no swarmer (completed) canceling does not produce exception.
3. Still no multinetwork support of course and thus removed senless parts of code.
|
Thu 08 Sep 2005 05:29:08 PM UTC, comment #26:
Hmm... code should be rewritten then :)
I guess to make it working we need to add swarmer filed into CommonFile.file
I'm not using multinet so i'm happy even with this patch, maybe someone else want to finish it
|
Thu 08 Sep 2005 04:20:06 PM UTC, comment #25:
+ if !found = 0 then Int64Swarmer.remove_swarmer file_swarmer
This is checking to see if it was found in current_files.
current_files is module specific.
If a swarmer is shared by one file in bittorrent, and one file in donkey, aren't you removing the swarmer from swarmers_by_name for the donkey file if you cancel the bittorrent file?
|
Thu 08 Sep 2005 03:26:31 PM UTC, comment #24:
Wrong, wrong, wrong.
>It isn't checking to see that the swarmer isn't in use by some other file.
Actually it does, that's the point. What do you think this is for?
+ if !found = 0 then Int64Swarmer.remove_swarmer file_swarmer
>Once you set file.file_swarmer <- None for every file that is using the particular swarmer, the garbage collector will automatically remove the entry from swarmers_by_name since no one else references it anymore.
If you have two files downloading, using the same swarmer, another file is referencing to same swarmer, even after cancelling one of them.
|
Thu 08 Sep 2005 03:18:16 PM UTC, comment #23:
What if you have two files downloading, using the same swarmer, and you cancel one of them? Won't your patch forcefully remove the swarmer and break the download of the other file? It isn't checking to see that the swarmer isn't in use by some other file.
The swarmers_by_name in commonSwarming2 is a Weak Hashtable. (a design decision of some sort)
Once you set file.file_swarmer <- None for every file that is using the particular swarmer, the garbage collector will automatically remove the entry from swarmers_by_name since no one else references it anymore.
Since you have to wait ~2 minutes after cancelling, this means the garbage collector (GC) didn't yet run.
If you add a call to "Gc.compact ();" in file_cancel (commonInteractive.ml after "impl.impl_file_ops.op_file_cancel impl.impl_file_val;" ) it should force the garbage collection immediately after a cancel. This is very ugly and slows down each call to file_cancel while garbage is collected.
You can test this theory by 1. starting a download, 2. cancelling it, 3. running "mem_stats -1", and then 4. trying to redownload the same file.
(without using your patch)
( mem_stats called with a negative integer parameter calls the garbage collector. )
|
Wed 07 Sep 2005 10:01:36 PM UTC, comment #22:
>Why is found in commonTypes? It doesn't belong.
>Why aren't you using List.exists instead of found?
Couse my Ocaml knowledge sucks
>There is duplication of code
Yes couse this code won't work from CommonSwarming.
>and it doesn't apply to all networks
Yes, couse i only care about bt/ed2k/filetp
>so this patch is probably wrong.
Well you might want to improove it in some way but it does it's job
|
Wed 07 Sep 2005 02:15:59 PM UTC, comment #21:
@2.6.4-broken-cancel.patch
Why is found in commonTypes? It doesn't belong.
Why aren't you using List.exists instead of found?
There is duplication of code, and it doesn't apply to all networks, so this patch is probably wrong.
|
Wed 07 Sep 2005 08:54:32 AM UTC, comment #20:
This one works fine
|
Wed 07 Sep 2005 07:56:15 AM UTC, comment #19:
Oh, well it's broken
|
Wed 07 Sep 2005 04:08:27 AM UTC, comment #18:
This patch finnaly solves the problem
I really wish i didn't have to write it
|
Tue 06 Sep 2005 10:27:50 PM UTC, comment #17:
Seems like with large number of downloads patch not doing it's job at all
|
Sun 21 Aug 2005 12:28:40 PM UTC, comment #16:
Applied your patch to CVS
|
Sun 21 Aug 2005 12:23:54 PM UTC, comment #15:
Copied your patch to patch #4334
|
Sun 21 Aug 2005 10:58:35 AM UTC, comment #14:
Well here is same patch for bt and filetp.
|
Wed 17 Aug 2005 12:45:51 AM UTC, comment #13:
Not sure if it can be closed, i just checked last 2.6.2 and "fast readding" problem is still there, and from what i remember it's just a symptom of several important problems
|
Sat 11 Jun 2005 07:53:10 PM UTC, comment #12:
Opened patch #4091 with the file cancel.patch from here.
|
Fri 10 Jun 2005 03:19:30 PM UTC, comment #11:
Did some tests, I don“t think "file.file_swarmer <- None;" will
help, the problem seems to be in commonFile. All tests where
done with a patched core.
Start the core in an empty dir, add an ed2k-link
mem_stats 1
Module CommonFile
files: 1
Module CommonSwarming2
Swarmers: 1
cancel the download
mem_stats 1
Module CommonFile
files: 1
Module CommonSwarming2
Swarmers: 1
10 seconds later
mem_stats 1
Module CommonFile
files: 1
Module CommonSwarming2
Swarmers: 0
add three other files and cancel them immediatly afterwards
mem_stats 1
Module CommonFile
files: 5
Module CommonSwarming2
Swarmers: 0
The swarmers took about two minutes to be cleaned, one swarmer
was removed immediatly after canceling, the two others took
the two minutes you mentioned, so this has nothing to do with
save_options_delay, weird...
But nonetheless in commonFile the files are never removed...
Although after save in files.ini there are no entries left, so
"file.file_swarmer <- None;" helped a bit indeed. I attached the
patch for it here.
|
Fri 10 Jun 2005 03:03:05 PM UTC, comment #10:
Well, i'm not sure about that - thing is, if you do manual save (even ten times), it won't clear the list.
Anyway, i think producing patch should come after fixing 2nd issue. Really hope to hear some wise adwise from someone like Amorphous :)
|
Fri 10 Jun 2005 02:56:10 PM UTC, comment #9:
You may be right about "file.file_swarmer <- None;"
from gnutellaClients.ml:
let download_finished file =
if List.memq file !current_files then begin
file_completed (as_file file);
file.file_swarmer <- None;
So the code is used there, too. This should be patched to have a
first solution. Regarding the postponed cleaning of the swarmer
list, I think it is related to save_options_delay, so it may be
longer than 2 minutes.
|
Fri 10 Jun 2005 10:16:49 AM UTC, comment #8:
Hmm, i remember myself checking commit code before, didn't find there anyhting userfull, but thnx for the great tip - i've checked some traces and found out, that in donkeyOneFile.ml in declare_completed_file function there is a code "file.file_swarmer <- None;" that can be used as i kind of solution.
Putiing this code into donkeyInteractive.ml into begining of file_ops.op_file_cancel function makes things working with one huge BUT.
if you start downloading, then cancel it immideatly file swarmer get's released, but list of swarmers still remembers it, if you start another donwnload immediatly it will tell you, you're the moron, as usually, but if you will restart core or wait 2 minutes list will get updated and everything will start working fine.
I've trying to fix this issue and located follwing code in commonSwarming2.ml
let swarmers =
define_option CommonComplexOptions.swarmers_section
["swarmers"] "All the swarmers used" (list_option SwarmerOption.t) []
and
let _ =
set_after_save_hook files_ini (fun _ -> swarmers =:= []);
set_before_save_hook files_ini (fun _ ->
let list = ref [] in
HS.iter (fun s ->
if s.s_networks <> [] then
list := s :: !list) swarmers_by_name;
swarmers =:= !list
);
set_after_load_hook files_ini (fun _ ->
List.iter (fun s ->
check_swarmer s;
) !!swarmers;
swarmers =:= [])
2nd part seems to be resposible for updating swarmers list, but list itself is unaccessable from any other part of code :(
Unfortunatly my ocaml knowledge is not strong enought to get the trick done, so i'd really apretiate help for one of big brothers.
|
Wed 08 Jun 2005 12:11:38 AM UTC, comment #7:
Note for devs: The swarmers section is removed when a file is committed. Maybe the necessary code section to be added to the
cancel command can be found in the commit code.
|
Fri 03 Jun 2005 04:50:04 PM UTC, comment #6:
Ok, i've discovered following: after starting download and then canceling it i can see this in files.ini
swarmers = [
{ file_size = 3926800
file_name = "./temp/urn_ed2k_AE9DEC30E5BC22F82061CEBB6A7D786B"
file_bitmap = 1
file_chunk_sizes = [
9728000;]
};]
i belive this should be removed on cancel, and in sources in file_ops.op_file_cancel i can't see anything like swarmer.remove. More than that, i can't find any similiar function in commonSwarming2.ml anyone can comment that?
|
Sun 29 May 2005 06:40:03 PM UTC, comment #5:
duplicate of savannah bug #12372
|
Fri 29 Apr 2005 10:05:15 AM UTC, comment #4:
or does it actually start a new download?
see savannah bug #12372 for what i reffered to in the last comment.
|
Fri 29 Apr 2005 09:18:02 AM UTC, comment #3:
afaik it's not a dublicate. this one is about that there remain traces of removed downloads and canceled downloads in mldonkey (until it is restarted). the other one is about exceptions when iterating networks, so the common code thinks that network (that didn't return false because of an exception) accepted the download and only starts the download as a non-primary in any following networks.
|
Thu 28 Apr 2005 12:35:32 PM UTC, comment #2:
Continuing in bug #12841 which is a somehow a duplicate.
|
Thu 28 Apr 2005 12:10:09 PM UTC, comment #1:
I could reproduce that bug:
start mlnet in a clean dir, lots of networks enabled:
Networks:
8 Donkey Enabled
7 BitTorrent Enabled
4 G2 Enabled
5 Fasttrack Disabled
2 Open Napster Enabled
3 Gnutella Enabled
6 FileTP Enabled
9 Soulseek Enabled
Add an ed2k download with dllink command, everything is normal.
Now cancel it via HTML interface and save ini files.
The file data in the swarmers[] section in files.ini is not
removed, the file in ./temp is removed.
Now re-enter the same dllink command:
"Exception File "src/daemon/common/commonSwarming2.ml", line 0, characters 609-14: Assertion failed for network Donkey
Exception Not_found for network FileTP
Unable to match URL"
the file in ./temp is created and files.ini is updated, but
not in the files[] section, instead of that a line has been
added to file_chunk_sizes, normally (after the first dllink)
it look like this:
file_chunk_sizes = [
9728000;]
after the second dllink with its error message it looks like this:
file_chunk_sizes = [
9728000;
9728000;]
|
Wed 20 Apr 2005 11:50:15 PM UTC, original submission:
If you add ed2k download, then cancel it and then add again, you will get
Exception Failure("File already downloaded (use 'force_download' if necessary)") for network Donkey
Exception Not_found for network FileTP
Unable to match URL
also on restart i've noticed that such downloads are recovered by temp_recover(so i guess problem is it's not eraiseing temp file)
Works fine in 2-5-28-1. After moving to any of 2-5-30 bug is showing it's face.
|