skynetservices / skydns1

DNS for skynet or any other service discovery
MIT License
528 stars 54 forks source link

Add no-expire flag to not auto evict records on ttl #84

Closed crosbymichael closed 10 years ago

crosbymichael commented 10 years ago

So there is a scaling issue with ttl and heartbeats. In the case of docker you can have 1000s of containers running and with a lower ttl in a dymanic environment it can become very chatty updating ttls for all the records. In the case of docker and skydock we know when a service is removed so there is no need for the overhead of heartbeats because docker will take care of that for us. Also there is a race trying to update 1000s of records that the heartbeat does not hit the cluster before the ttl is automatically expired.

Let me know what you all think about this or if you know of a better way.

miekg commented 10 years ago

Two things, one on the architecture and one on the implemention. To start with the first:

I think it is nicer to have an extra json element where a client can specify a no-expire. This means that you can have mixed expire and no-expire clients.

Two: I had doubts skydns could scale into the 10000's, but already seeing problems with 1000s of clients is troublesome. I do think SkyDNS can be made more efficient, but this will not buy us that much. Making SkyDNS completely callback driven is (DNS and HTTP callbacks) is much better, but requires client side changes (which for skydock might not be that bad)

bketelsen commented 10 years ago

I really like the idea of no-expire. It fits the docker case well and allows for less work on SkyDNS side.

crosbymichael commented 10 years ago

@miekg well we can make a few changes to handle more clients. I'm not totally sure what you mean by the callback approach. I noticed that skydns has a http callback but I have not used it and do not know what the usecase is for.

I have also been testing different backends with skydns ( redis ) to see if we can make it handle more services and it works well.

crosbymichael commented 10 years ago

So what do you all think? ;)

miekg commented 10 years ago

I think using a TTL of -1 makes most sense. This allows for both expire and non-expire names. Also such a change should be trivial to implement. On 2 Apr 2014 07:46, "Michael Crosby" notifications@github.com wrote:

So what do you all think? ;)

— Reply to this email directly or view it on GitHubhttps://github.com/skynetservices/skydns/pull/84#issuecomment-39293663 .

crosbymichael commented 10 years ago

@miekg yes, but you still have the overhead of the ticker running and you want skydns to return a ttl but not expire records internally. I.E. I still want skydns to return a ttl of 5s but not expire records.

Is a -1 ttl valid in DNS? It may work if we don't break things.

miekg commented 10 years ago

TTL is DNS is a postive number. It may be 0.

I see what you mean about the TTL you do want to return. This would a flag in the json.

I'm not completely against your patch, but I dislike the all or nothing element of it.

crosbymichael commented 10 years ago

@miekg i understand. What is an alternative? Not expire for a specific environment or domain?

miekg commented 10 years ago

Why can't a client add an (optional) noexpire bool to the initial json registering the name?

crosbymichael commented 10 years ago

Updated to be record specific

miekg commented 10 years ago

LGTM. Maybe add a little test?