[Twisted-Python] twisted.names and multicast DNS
So after two years of leaving it alone, I decided to have another look at the multicast DNS module I was writing for Twisted, and lo and behold I discover a serious design flaw in my response cache. Here's the basic situation I'm dealing with: Suppose I have two hosts on my network, called ford and arthur. I'm sitting at arthur, and I'm looking for SSH servers advertised via DNS-based Service Discovery and Multicast DNS. At the present moment, ford is switched off. I turn on ford, and when the SSH server starts up, ford broadcasts an announcement DNS datagram that says, "In the zone '_ssh._tcp.local', there is a PTR record with the data 'Ford._ssh._tcp.local'. This information expires in 7200 seconds." (it mentions the class too, of course, but the class doesn't affect the discussion below so we'll ignore it from here on). arthur receives the announcement, records the transmission time and the TTL, and caches the PTR record. Thereafter, any program on arthur that requests PTR records in '_ssh._tcp.local' will be given the PTR record about 'Ford._ssh._tcp.local'. Now I turn off Ford, and when the SSH server shuts down, ford broadcasts a goodbye datagram that says, "In the zone '_ssh._tcp.local', there is a PTR record with the data 'Ford._ssh._tcp.local'. This information expires in 0 seconds." - that is to say, exactly the same as the previous announcement but with a TTL of 0. The cache on arthur needs to compare this new record to its cache, see that it matches the definition of an existing record, and update the TTL of the existing record so that it will expire immediately. I'm trying to implement the record cache on arthur at the moment. It needs to be queried by (name, type, class) tuples, so I use such a tuple as the key of a dictionary. Since there can be multiple records available for a particular (name, type, class), the value associated with this key is a list of resource records. Because the t.names.dns.DNSDatagramProtocol gives me instances of the various t.names.dns.Record_* classes, I figured that those instances would be a good representation of the resource records I need to store. In psuedo-python, then, the ideal data structure would look like this: cache = { ('_ssh._tcp.local', PTR, IN): [ (<Record_PTR name=Ford._ssh._tcp.local>, 7200), ] } ...then, when I get a new record from the network, I can look up its (name, type, class) tuple and get the list of records. Then I compare the new record to each of the records in the list: if it matches a record I update the TTL, otherwise I add the record to the list. The problem is this: the Record_PTR class (just like all the other Record_* classes) in t.names.dns stores the TTL as an instance variable! Therefore, when I have an existing record and an updated record, they never match, and I wind up with two identical records in my data structure that differ only by TTL. As far as I can tell, I have three options from here: 1. Write my own versions of all the Record_* classes that don't maintain a TTL value, and translate backwards and forwards whenever I need to talk to Twisted's DNS layer. I'd need to update this code whenever a new DNS record type was added. 2. Write a comparison function for comparing resource records that compares instance variables that aren't named 'ttl'. This would probably need to be updated less frequently, but is horribly hacky. 3. Complain on the twisted-list and hope that somebody has a better suggestion, or changes the code to be more mDNS-friendly. As you can see, I'm trying Option 3 at the moment, but I'm open to suggestions. :)
On 7/9/05, Tim Allen <screwtape@froup.com> wrote you forgot option number 4, which is what we ended up doing. Put a python binding in over the Apple code and use it integrated into Twisted. Plus you get to use it outside Twisted as well :-) -- If you don't know what you want, you probably need a nap.
On Sat, 9 Jul 2005 17:08:15 +1000, Tim Allen <screwtape@froup.com> wrote:
So after two years of leaving it alone, I decided to have another look at the multicast DNS module I was writing for Twisted, and lo and behold I discover a serious design flaw in my response cache. Here's the basic situation I'm dealing with:
[snip]
The problem is this: the Record_PTR class (just like all the other Record_* classes) in t.names.dns stores the TTL as an instance variable! Therefore, when I have an existing record and an updated record, they never match, and I wind up with two identical records in my data structure that differ only by TTL.
As far as I can tell, I have three options from here:
1. Write my own versions of all the Record_* classes that don't maintain a TTL value, and translate backwards and forwards whenever I need to talk to Twisted's DNS layer. I'd need to update this code whenever a new DNS record type was added. 2. Write a comparison function for comparing resource records that compares instance variables that aren't named 'ttl'. This would probably need to be updated less frequently, but is horribly hacky.
What's hacky about this? It seems like a simple, straightforward solution. Record classes have comparison defined one way, you want to compare them another way. Comparison can only be defined one way using the "==" operator, so clearly one of these requirements has to resort to using something other than the "==" operator. I think it's even nicer than you suggest, since the attributes which should be compared are all declared on each class. eg, Record_NS.compareAttributes == ('name', 'ttl'). You should be able to write a comparison function that looks at this attribute and simply omits ttl. Hope this helps, Jp
On 11 Jul 2005, at 04:16, Jp Calderone wrote:
What's hacky about this? It seems like a simple, straightforward solution. Record classes have comparison defined one way, you want to compare them another way. Comparison can only be defined one way using the "==" operator, so clearly one of these requirements has to resort to using something other than the "==" operator.
Well, it would be nicer still if the world would conform to my requirements. :) I was sort of hoping that someone would pop their head out of the woodwork and say 'No, it was designed this way because it makes such-and-such a task much easier', or even 'Nobody much cares either way, we're taking patches'.
I think it's even nicer than you suggest, since the attributes which should be compared are all declared on each class. eg, Record_NS.compareAttributes == ('name', 'ttl'). You should be able to write a comparison function that looks at this attribute and simply omits ttl.
I was a bit nervous about relying on undocumented instance variables of a module marked as 'unstable', but I suppose since I'm already relying on an unstable module, one more assumption isn't going to hurt. Thanks!
participants (3)
-
jarrod roberson -
Jp Calderone -
Tim Allen