Slightly worse, in that, the code only ever uses the first IP returned from the DNS lookups it initiates. Some domains may return 16 IPs but that code would use only the first, at least until the cached-entry expires.
If I recall correctly, that's how the Node maintainers want it in the core to...
I was investigating errors in node code for a few weeks, and traced it to DNS returning a bad IP address. Node didn't even try to call the second IP address, it just returned a "request made but no response" error
Looked deeper, and saw other similar complaints asking for nice to support failover and try the second IP, but it was highly opposed. They suggested writing your own DNS handling code (which is exactly what this person is doing, though for a different reason)
The main issue in the article are calls to getaddrinfo() on every send which create a delay, which is not only DNS related. The function does DNS lookups but also translates IP address strings to address data that can be used by the kernel (e.g. the different ways to write IPv6 addresses like “fe80::1%eth0”)
I haven't read the article yet but the problem I had back then running into UDP with DNS on K8S is that it might consume the UV thread pool and exhausted it.
There is only 4 UV threadpool, and DNS resolution use it. When using service with low ttl or if for wahtever reason bug or something and if node has to resolve DNS every time under highload, it's very quickly consume all of thoese 4 UV thread pool.
If you're reimplementing DNS lookups with caching in your own code, you are very likely doing something wrong.
The title of the entire story already had me confused, as it suggests you somehow need DNS for UDP. Everything that is presented here is a caching strategy.
Maybe not everybody cares about the wasted tick, but the response to such niche feature requests in open source projects used to be a simple “patches welcome”. I found that way more helpful and less aggressive than what seems to be the new default in Github projects, “won’t fix”. I don’t understand this shift.
Maintainers have learned that "patches welcome" is anything but simple. You need to integrate others' patches, make sure they're tested, and make sure they don't break despite probably not having anyone in the core team (which might just be you!) who uses those features.
Counterpoint: i think "Won't fix, don't understand" may be the responsible thing to do when you, as a single maintainer, can't give everybody's new ideas a thorough check before merging. YMMV.
An alternative approach would be to call "await util.promisify(socket.connect)(8125, host)" so that subsequent calls to send don't need to name the destination (and the underlying runtime could call send instead of sendto). Perhaps the author needs the messages to be sent to a new host quickly when DNS records change, but the trade-off might be different depending on the use case, and perhaps some applications might prefer to periodically re-connect the socket to re-resolve DNS.
Looking at the UDP code in question in brightcove/hot-shots [1]. The createUdpTransport() function creates an object with a send() method that forwards to socket.send(), calling the underlying function with the same host and port argument each time. It seems to me that this code should call socket.connect(args.host, args.port) once instead, and skip the host and port arguments to the send() calls. Presumably this would result in just a single DNS lookup.
Bonus: the createUdpTransport() function also has a DNS cache, which the author of the article could have enabled with just "cacheDns: true". But if the library code used socket.connect(), this internal cache shouldn't be needed either.
> If the socket sockfd is of type SOCK_DGRAM, then addr is the address to which datagrams are sent by default, and the only address from which datagrams are received.
After that, you can use send() instead of sendmsg(), so it saves you on having to pass the host on each call to sendmsg.
Same in Java. For both TCP and UDP, you build a socket and write to that.
Actually, I don't know what happens if the DNS record expires during the connection. I would presume nothing -- that the socket will stay open with the same (possibly now invalid) destination IP until closed.
24 comments
[ 4.9 ms ] story [ 62.9 ms ] threadEverything else remains pretty much the same...
Slightly worse, in that, the code only ever uses the first IP returned from the DNS lookups it initiates. Some domains may return 16 IPs but that code would use only the first, at least until the cached-entry expires.
I was investigating errors in node code for a few weeks, and traced it to DNS returning a bad IP address. Node didn't even try to call the second IP address, it just returned a "request made but no response" error
Looked deeper, and saw other similar complaints asking for nice to support failover and try the second IP, but it was highly opposed. They suggested writing your own DNS handling code (which is exactly what this person is doing, though for a different reason)
As another commented pointed out the "solution" only uses a single IP, even if the resolver returned multiple.
I would not use this in production.
There is only 4 UV threadpool, and DNS resolution use it. When using service with low ttl or if for wahtever reason bug or something and if node has to resolve DNS every time under highload, it's very quickly consume all of thoese 4 UV thread pool.
The title of the entire story already had me confused, as it suggests you somehow need DNS for UDP. Everything that is presented here is a caching strategy.
In this case it is probably 'using the wrong tool, Node.js'. [0]
> The title of the entire story already had me confused
I needed to read the whole article two times to understand what exactly is going on.
It's really should be titled "Sending UDP Messages a bit faster in Node.js because Node.js by default sucks at invoking getaddrinfo()"
[0] https://youtu.be/v79fYnuVzdI
Bonus: the createUdpTransport() function also has a DNS cache, which the author of the article could have enabled with just "cacheDns: true". But if the library code used socket.connect(), this internal cache shouldn't be needed either.
1: https://github.com/brightcove/hot-shots/blob/564ac7bfdaaf366...
> If the socket sockfd is of type SOCK_DGRAM, then addr is the address to which datagrams are sent by default, and the only address from which datagrams are received.
After that, you can use send() instead of sendmsg(), so it saves you on having to pass the host on each call to sendmsg.
It can also avoid extra DNS lookups: everything is handled for you by the kernel. See also: https://stackoverflow.com/a/51296247
Actually, I don't know what happens if the DNS record expires during the connection. I would presume nothing -- that the socket will stay open with the same (possibly now invalid) destination IP until closed.
https://github.com/brightcove/hot-shots/issues/223
I don't see `send2` in the node docs[2].
[1] https://beej.us/guide/bgnet/html/#sendtorecv [2] https://nodejs.org/api/dgram.html