Skip to content

Commit c754d12

Browse files
orgadsorgads
committed
dns: coalesce identical concurrent lookup() requests
When multiple callers issue dns.lookup() for the same (hostname, family, hints, order) concurrently, only one getaddrinfo call is now dispatched to the libuv threadpool. All callers share the result. getaddrinfo is a blocking call that runs on the libuv threadpool (capped at 4 threads by default, with a slow I/O concurrency limit of 2). When DNS resolution is slow - e.g. ~10-20 s per call due to a misbehaving resolver - identical requests queue behind each other, causing timeouts that grow linearly with the number of concurrent callers: Before: 100 parallel lookup('host') -> 50 batches × 10 s = 500+ s After: 100 parallel lookup('host') -> 1 getaddrinfo call = ~10 s This is particularly severe on WSL, where the DNS relay rewrites QNAMEs in responses (appending the search domain), causing glibc to discard them as non-matching and wait for a 5s timeout per retry. The coalescing is keyed on (hostname, family, hints, order) so lookups with different options still get separate getaddrinfo calls. Each caller independently post-processes the shared raw result (applying the 'all' flag, constructing address objects, etc.). Signed-off-by: Orgad Shaneh <orgad.shaneh@audiocodes.com> PR-URL: nodejs#62599 Fixes: nodejs#62503
1 parent dec5973 commit c754d12

File tree

4 files changed

+311
-105
lines changed

4 files changed

+311
-105
lines changed

‎lib/dns.js‎

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
'use strict';
2323

2424
const {
25+
ArrayPrototypePush,
2526
ObjectDefineProperties,
2627
ObjectDefineProperty,
28+
SafeMap,
2729
Symbol,
2830
} = primordials;
2931

@@ -105,36 +107,12 @@ const {
105107

106108
let promises = null; // Lazy loaded
107109

108-
function onlookup(err, addresses) {
109-
if (err) {
110-
return this.callback(new DNSException(err, 'getaddrinfo', this.hostname));
111-
}
112-
this.callback(null, addresses[0], this.family || isIP(addresses[0]));
113-
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
114-
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
115-
}
116-
}
117-
118-
119-
function onlookupall(err, addresses) {
120-
if (err) {
121-
return this.callback(new DNSException(err, 'getaddrinfo', this.hostname));
122-
}
123-
124-
const family = this.family;
125-
for (let i = 0; i < addresses.length; i++) {
126-
const addr = addresses[i];
127-
addresses[i] = {
128-
address: addr,
129-
family: family || isIP(addr),
130-
};
131-
}
132-
133-
this.callback(null, addresses);
134-
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
135-
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
136-
}
137-
}
110+
// Map of in-flight getaddrinfo requests for lookup coalescing.
111+
// Key: `${hostname}\0${family}\0${hints}\0${order}`
112+
// Value: Array<{ callback, all }>
113+
// When multiple callers request the same lookup concurrently, only one
114+
// getaddrinfo call is dispatched to the libuv threadpool.
115+
const inflightLookups = new SafeMap();
138116

139117

140118
// Easy DNS A/AAAA look up
@@ -213,12 +191,6 @@ function lookup(hostname, options, callback) {
213191
return {};
214192
}
215193

216-
const req = new GetAddrInfoReqWrap();
217-
req.callback = callback;
218-
req.family = family;
219-
req.hostname = hostname;
220-
req.oncomplete = all ? onlookupall : onlookup;
221-
222194
let order = DNS_ORDER_VERBATIM;
223195

224196
if (dnsOrder === 'ipv4first') {
@@ -227,10 +199,53 @@ function lookup(hostname, options, callback) {
227199
order = DNS_ORDER_IPV6_FIRST;
228200
}
229201

202+
const key = `${hostname}\0${family}\0${hints}\0${order}`;
203+
const waiter = { callback, all };
204+
205+
let inflight = inflightLookups.get(key);
206+
if (inflight) {
207+
// Piggyback on existing in-flight request.
208+
ArrayPrototypePush(inflight, waiter);
209+
return {};
210+
}
211+
212+
// First request for this key — dispatch to libuv.
213+
inflight = [waiter];
214+
inflightLookups.set(key, inflight);
215+
216+
const req = new GetAddrInfoReqWrap();
217+
req.family = family;
218+
req.hostname = hostname;
219+
req.oncomplete = function(errCode, addresses) {
220+
inflightLookups.delete(key);
221+
const waiters = inflight;
222+
for (let i = 0; i < waiters.length; i++) {
223+
const w = waiters[i];
224+
if (errCode) {
225+
w.callback(new DNSException(errCode, 'getaddrinfo', hostname));
226+
} else if (w.all) {
227+
const result = new Array(addresses.length);
228+
for (let j = 0; j < addresses.length; j++) {
229+
result[j] = {
230+
address: addresses[j],
231+
family: family || isIP(addresses[j]),
232+
};
233+
}
234+
w.callback(null, result);
235+
} else {
236+
w.callback(null, addresses[0], family || isIP(addresses[0]));
237+
}
238+
}
239+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
240+
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
241+
}
242+
};
243+
230244
const err = cares.getaddrinfo(
231245
req, hostname, family, hints, order,
232246
);
233247
if (err) {
248+
inflightLookups.delete(key);
234249
process.nextTick(callback, new DNSException(err, 'getaddrinfo', hostname));
235250
return {};
236251
}

‎lib/internal/dns/promises.js‎

Lines changed: 62 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const {
44
FunctionPrototypeCall,
55
ObjectDefineProperty,
66
Promise,
7+
SafeMap,
78
Symbol,
89
} = primordials;
910

@@ -82,41 +83,14 @@ const {
8283
stopPerf,
8384
} = require('internal/perf/observe');
8485

85-
function onlookup(err, addresses) {
86-
if (err) {
87-
this.reject(new DNSException(err, 'getaddrinfo', this.hostname));
88-
return;
89-
}
90-
91-
const family = this.family || isIP(addresses[0]);
92-
this.resolve({ address: addresses[0], family });
93-
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
94-
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
95-
}
96-
}
97-
98-
function onlookupall(err, addresses) {
99-
if (err) {
100-
this.reject(new DNSException(err, 'getaddrinfo', this.hostname));
101-
return;
102-
}
103-
104-
const family = this.family;
105-
106-
for (let i = 0; i < addresses.length; i++) {
107-
const address = addresses[i];
108-
109-
addresses[i] = {
110-
address,
111-
family: family || isIP(addresses[i]),
112-
};
113-
}
114-
115-
this.resolve(addresses);
116-
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
117-
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
118-
}
119-
}
86+
// Map of in-flight getaddrinfo requests for lookup coalescing.
87+
// Key: `${hostname}\0${family}\0${hints}\0${order}`
88+
// Value: Promise<{ err: number|null, addresses: string[]|null }>
89+
// When multiple callers request the same lookup concurrently, only one
90+
// getaddrinfo call is dispatched to the libuv threadpool. All callers
91+
// share the result, avoiding threadpool starvation under sustained DNS
92+
// failure where each getaddrinfo blocks for many seconds.
93+
const inflightLookups = new SafeMap();
12094

12195
/**
12296
* Creates a promise that resolves with the IP address of the given hostname.
@@ -132,41 +106,48 @@ function onlookupall(err, addresses) {
132106
* @property {0 | 4 | 6} family - The IP address type. 4 for IPv4 or 6 for IPv6, or 0 (for both).
133107
*/
134108
function createLookupPromise(family, hostname, all, hints, dnsOrder) {
135-
return new Promise((resolve, reject) => {
136-
if (!hostname) {
137-
reject(new ERR_INVALID_ARG_VALUE('hostname', hostname,
138-
'must be a non-empty string'));
139-
return;
140-
}
109+
if (!hostname) {
110+
return Promise.reject(
111+
new ERR_INVALID_ARG_VALUE('hostname', hostname,
112+
'must be a non-empty string'));
113+
}
141114

142-
const matchedFamily = isIP(hostname);
115+
const matchedFamily = isIP(hostname);
116+
if (matchedFamily !== 0) {
117+
const result = { address: hostname, family: matchedFamily };
118+
return Promise.resolve(all ? [result] : result);
119+
}
143120

144-
if (matchedFamily !== 0) {
145-
const result = { address: hostname, family: matchedFamily };
146-
resolve(all ? [result] : result);
147-
return;
148-
}
121+
let order = DNS_ORDER_VERBATIM;
122+
if (dnsOrder === 'ipv4first') {
123+
order = DNS_ORDER_IPV4_FIRST;
124+
} else if (dnsOrder === 'ipv6first') {
125+
order = DNS_ORDER_IPV6_FIRST;
126+
}
149127

150-
const req = new GetAddrInfoReqWrap();
128+
const key = `${hostname}\0${family}\0${hints}\0${order}`;
151129

130+
let rawPromise = inflightLookups.get(key);
131+
if (!rawPromise) {
132+
let resolveRaw;
133+
rawPromise = new Promise((resolve) => { resolveRaw = resolve; });
134+
inflightLookups.set(key, rawPromise);
135+
136+
const req = new GetAddrInfoReqWrap();
152137
req.family = family;
153138
req.hostname = hostname;
154-
req.oncomplete = all ? onlookupall : onlookup;
155-
req.resolve = resolve;
156-
req.reject = reject;
157-
158-
let order = DNS_ORDER_VERBATIM;
159-
160-
if (dnsOrder === 'ipv4first') {
161-
order = DNS_ORDER_IPV4_FIRST;
162-
} else if (dnsOrder === 'ipv6first') {
163-
order = DNS_ORDER_IPV6_FIRST;
164-
}
165-
166-
const err = getaddrinfo(req, hostname, family, hints, order);
139+
req.oncomplete = function(errCode, addresses) {
140+
inflightLookups.delete(key);
141+
resolveRaw({ err: errCode, addresses });
142+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
143+
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
144+
}
145+
};
167146

168-
if (err) {
169-
reject(new DNSException(err, 'getaddrinfo', hostname));
147+
const errCode = getaddrinfo(req, hostname, family, hints, order);
148+
if (errCode) {
149+
inflightLookups.delete(key);
150+
resolveRaw({ err: errCode, addresses: null });
170151
} else if (hasObserver('dns')) {
171152
const detail = {
172153
hostname,
@@ -177,6 +158,24 @@ function createLookupPromise(family, hostname, all, hints, dnsOrder) {
177158
};
178159
startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
179160
}
161+
}
162+
163+
// Each caller post-processes the shared raw result independently.
164+
return rawPromise.then(({ err, addresses }) => {
165+
if (err) {
166+
throw new DNSException(err, 'getaddrinfo', hostname);
167+
}
168+
if (all) {
169+
const result = new Array(addresses.length);
170+
for (let i = 0; i < addresses.length; i++) {
171+
result[i] = {
172+
address: addresses[i],
173+
family: family || isIP(addresses[i]),
174+
};
175+
}
176+
return result;
177+
}
178+
return { address: addresses[0], family: family || isIP(addresses[0]) };
180179
});
181180
}
182181

0 commit comments

Comments
 (0)