aMule Forum
English => en_Bugs => Topic started by: Stu Redman on April 01, 2008, 12:06:28 AM
-
I've seen a few times that aMule killed my nodes.dat. :o It was either completely empty (4 bytes), or very small, so that at the next start KAD boot failed.
So I added a sanity check: write file only if a minimum of 50 contacts is available. Maximum is 200, therefore I think 50 could be a sensible threshold.
To make the KAD boot really reliable more drastic measures would be necessary:
- for each contact, store date when it was first seen under that IP, and when last (change of file format!)
- prefer storing contacts with static adresses (available for a long time) to dynamic contacts. Don't drop static contacts at once if they were just one time not there.
I don't know if that's really necessary, but it might do away with the KAD booting problems.
Index: src/kademlia/routing/RoutingZone.cpp
===================================================================
--- src/kademlia/routing/RoutingZone.cpp (revision 8312)
+++ src/kademlia/routing/RoutingZone.cpp (working copy)
@@ -154,9 +154,11 @@
unsigned int count = 0;
CContact *c;
CFile file;
- if (file.Open(m_filename, CFile::write)) {
- ContactList contacts;
- GetBootstrapContacts(&contacts, 200);
+ ContactList contacts;
+ int found = GetBootstrapContacts(&contacts, 200);
+ if ( found < 50) {
+ AddDebugLogLineM( false, logKadRouting, wxString::Format(wxT("Only %d contactsavailable, file not written."), count));
+ } else if (file.Open(m_filename, CFile::write)) {
file.WriteUInt32((uint32)std::min((int)contacts.size(), CONTACT_FILE_LIMIT));
ContactList::const_iterator it;
for (it = contacts.begin(); it != contacts.end(); ++it) {
-
Format can't be changed.
-
Why ? It's not compatible with eMule anyway. :P
Besides, the little patch above changes nothing in the format.
aMule:
file.WriteUInt32((uint32)std::min((int)contacts.size(), CONTACT_FILE_LIMIT));
ContactList::const_iterator it;
for (it = contacts.begin(); it != contacts.end(); ++it) {
count++;
c = *it;
file.WriteUInt128(c->GetClientID());
file.WriteUInt32(c->GetIPAddress());
file.WriteUInt16(c->GetUDPPort());
file.WriteUInt16(c->GetTCPPort());
file.WriteUInt8(c->GetType());
if (count == CONTACT_FILE_LIMIT) {
break;
}
}
eMule 0.48a:
// Start file with 0 to prevent older clients from reading it.
file.WriteUInt32(0);
// Now tag it with a version which happens to be 1.
file.WriteUInt32(1);
file.WriteUInt32((uint32)listContacts.size());
for (ContactList::const_iterator itContactList = listContacts.begin(); itContactList != listContacts.end(); ++itContactList)
{
CContact* pContact = *itContactList;
pContact->GetClientID(&uID);
file.WriteUInt128(&uID);
file.WriteUInt32(pContact->GetIPAddress());
file.WriteUInt16(pContact->GetUDPPort());
file.WriteUInt16(pContact->GetTCPPort());
file.WriteUInt8(pContact->GetVersion());
}
-
It is compatible with emule, actually. Kinda.
-
"Kinda" meaning, eMule can read aMule's nodes.dat and aMule can't read eMule's. Now I know what the nodes.dat posts in the problem forum are about. ;D
I've extended the fix so aMule can now read eMule's nodes.dat too. Now the usability of downloaded nodes.dats should rise from 2% to 100%. ;)
Still no change in the file format.
-
Your patch makes aMule an incompatible client for the Kademlia network.
Sorry, but that's just not that easy.
-
Huh ? ???
There are two parts in the patch:
1) allow loading of nodes.dat in eMule format (and of course existing files from aMule)
2) adding sanity check before saving (of course you could lower the 50 contact limit I set)
Now how does this make aMule an incompatible client for the Kademlia network? It is connecting fine to Kad here with the patch.
as it is now
1) aMule doesn't load nodes.dat from eMule (no of entries is wrongly interpreted as 0)
2) aMule happily writes empty nodes.dat under some conditions (like after startup with a hung up router and a quick exit). THIS effectively cuts the client off Kad, and 1) makes it very hard to get back because downloading a nodes.dat is pointless.
If I'm overlooking something please point it out for me. Just "breaks aMule" is not helpful, no matter how often you repeat it. ::)
-
Ok, then after being able to read the file, aMule must also be able to interpret its contents properly. That's what is missing from your patch.
-
The content is identical (see comparison of sources above), except that the new eMule Format has a leading 00000000 00000001.
The code of the patch is original eMule code.
I have run patched aMule with eMule nodes.dat RIGHT NOW and it read the contacts and connected just fine.
I am still not getting your point.
-
~10 lines below the first fragment you changed (reading the file):
uint8 uContactVersion = 0;
byte byType = 0;
if(uVersion == 1)
uContactVersion = file.ReadUInt8();
else
byType = file.ReadUInt8();
and everything that it implies.
-
You are right. Well, you could have explained the problem in your first post instead of sphinxing around the way you did. ::)
Back to post one. You implemented the nodes.dat loader, but the WriteFile is still without sanity check.
-
Anyway, the important bug fixing here is on part 2:
Yes.
Sturednan , could you post a stripped version of you patch fixing sanity checks on nodes.dat file writes only?
I believe GonoszTopy shouldn't be against committing that.
It's in the first post of the thread. And if anybody agreed with us and found it worth commiting, they would have done it. :-X
-
BtW: it just wrote a 12 byte nodes.dat (http://forum.amule.org/index.php?topic=14714.0) AGAIN. Do you like it that way ? ::)
Do you like to store dead contacts?
-
Noticed the same problem: only 12 bytes of nodes.dat (I suppose the 12 bytes of header).
Probably it should be better to understand WHY the contacts list becomes empty.
In the meanwhile, IMHO, it's "preferable" to have a OLD nodes.dat file (even if it contains dead contacts) instead of a zero contact file: the zero contact file (12 bytes only nodes.dat) causes to my aMule to remain in "Contacting..." status.
Bye,
Mr Hyde
-
Do you like to store dead contacts?
No. But how do you get the idea they are "dead" ?
I tell you what I did. I started aMule twice, each time checking the directory tree in the settings menu, and then closed it again. Each time KAD connected fine meanwhile. Third time I started it, it did NOT connect at all (Kad Graph stayed at zero), and nodes.dat was empty.
Do you truely want to consider any contact as "dead" after only 30s of uptime ?
I don't know what exactly causes this misconsideration, but writing a zero file just makes no sense. I'd go as far to rather keep 3k of "dead" contacts along with 1k of "live" ones than just the live ones. Maybe they are really just fine and just a half hung up router caused them to be considered as dead.
As it is now, every user has to manually back up his nodes.dat (the LOG of all things is backed up, but not the nodes.dat ::) ) or risk being cut off and go hunting for a download (which at least works now again). I call this a flaw.
-
Or just download a new list. Which is what they should do.
-
Which will be useless again once eMule 0.49 comes out and all lists will be in its format.
Why are you all so opposed against a little sanity check which would do away with the whole problem ?
-
Problem still there.
Also I just found it had been sitting there dumb and disconnected for 27 hours. >:(
There should be some automatic restart after a certain time of total inactivity.
-
So let's ask again - is it ok to add this sanity check ? And is the limit of 50 ok or should I lower it to 30 ?
I've had it running like this for months and it's saving my nodes.dat whenever my router hangs up. :D
-
I can agree with it, but change:
- contacts.dat to nodes.dat
- wxT() to wxPLURAL (http://docs.wxwidgets.org/stable/wx_stringfunctions.html#wxplural)
I'd also set the limit as 25, 50 seems too high to me.
-
Done. :D