Skip to content

Commit b213b0f

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: #62599 Fixes: #62503
1 parent dec5973 commit b213b0f

File tree

4 files changed

+313
-105
lines changed

4 files changed

+313
-105
lines changed

‎lib/dns.js‎

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

2424
const {
25+
Array,
26+
ArrayPrototypePush,
2527
ObjectDefineProperties,
2628
ObjectDefineProperty,
29+
SafeMap,
2730
Symbol,
2831
} = primordials;
2932

@@ -105,36 +108,12 @@ const {
105108

106109
let promises = null; // Lazy loaded
107110

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-
}
111+
// Map of in-flight getaddrinfo requests for lookup coalescing.
112+
// Key: `${hostname}\0${family}\0${hints}\0${order}`
113+
// Value: Array<{ callback, all }>
114+
// When multiple callers request the same lookup concurrently, only one
115+
// getaddrinfo call is dispatched to the libuv threadpool.
116+
const inflightLookups = new SafeMap();
138117

139118

140119
// Easy DNS A/AAAA look up
@@ -213,12 +192,6 @@ function lookup(hostname, options, callback) {
213192
return {};
214193
}
215194

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

224197
if (dnsOrder === 'ipv4first') {
@@ -227,10 +200,53 @@ function lookup(hostname, options, callback) {
227200
order = DNS_ORDER_IPV6_FIRST;
228201
}
229202

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

‎lib/internal/dns/promises.js‎

Lines changed: 65 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
'use strict';
22
const {
3+
Array,
34
ArrayPrototypeMap,
45
FunctionPrototypeCall,
56
ObjectDefineProperty,
67
Promise,
8+
PromiseReject,
9+
PromiseResolve,
10+
SafeMap,
711
Symbol,
812
} = primordials;
913

@@ -82,41 +86,14 @@ const {
8286
stopPerf,
8387
} = require('internal/perf/observe');
8488

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-
}
89+
// Map of in-flight getaddrinfo requests for lookup coalescing.
90+
// Key: `${hostname}\0${family}\0${hints}\0${order}`
91+
// Value: Promise<{ err: number|null, addresses: string[]|null }>
92+
// When multiple callers request the same lookup concurrently, only one
93+
// getaddrinfo call is dispatched to the libuv threadpool. All callers
94+
// share the result, avoiding threadpool starvation under sustained DNS
95+
// failure where each getaddrinfo blocks for many seconds.
96+
const inflightLookups = new SafeMap();
12097

12198
/**
12299
* Creates a promise that resolves with the IP address of the given hostname.
@@ -132,41 +109,48 @@ function onlookupall(err, addresses) {
132109
* @property {0 | 4 | 6} family - The IP address type. 4 for IPv4 or 6 for IPv6, or 0 (for both).
133110
*/
134111
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-
}
112+
if (!hostname) {
113+
return PromiseReject(
114+
new ERR_INVALID_ARG_VALUE('hostname', hostname,
115+
'must be a non-empty string'));
116+
}
141117

142-
const matchedFamily = isIP(hostname);
118+
const matchedFamily = isIP(hostname);
119+
if (matchedFamily !== 0) {
120+
const result = { address: hostname, family: matchedFamily };
121+
return PromiseResolve(all ? [result] : result);
122+
}
143123

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

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

133+
let rawPromise = inflightLookups.get(key);
134+
if (!rawPromise) {
135+
let resolveRaw;
136+
rawPromise = new Promise((resolve) => { resolveRaw = resolve; });
137+
inflightLookups.set(key, rawPromise);
138+
139+
const req = new GetAddrInfoReqWrap();
152140
req.family = family;
153141
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);
142+
req.oncomplete = function(errCode, addresses) {
143+
inflightLookups.delete(key);
144+
resolveRaw({ err: errCode, addresses });
145+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
146+
stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });
147+
}
148+
};
167149

168-
if (err) {
169-
reject(new DNSException(err, 'getaddrinfo', hostname));
150+
const errCode = getaddrinfo(req, hostname, family, hints, order);
151+
if (errCode) {
152+
inflightLookups.delete(key);
153+
resolveRaw({ err: errCode, addresses: null });
170154
} else if (hasObserver('dns')) {
171155
const detail = {
172156
hostname,
@@ -177,6 +161,24 @@ function createLookupPromise(family, hostname, all, hints, dnsOrder) {
177161
};
178162
startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
179163
}
164+
}
165+
166+
// Each caller post-processes the shared raw result independently.
167+
return rawPromise.then(({ err, addresses }) => {
168+
if (err) {
169+
throw new DNSException(err, 'getaddrinfo', hostname);
170+
}
171+
if (all) {
172+
const result = new Array(addresses.length);
173+
for (let i = 0; i < addresses.length; i++) {
174+
result[i] = {
175+
address: addresses[i],
176+
family: family || isIP(addresses[i]),
177+
};
178+
}
179+
return result;
180+
}
181+
return { address: addresses[0], family: family || isIP(addresses[0]) };
180182
});
181183
}
182184

0 commit comments

Comments
 (0)