Skip to content

DNS interceptor does not deduplicate concurrent lookups for the same hostname #5974

Open

Description

@kholstinin

Bug Description

interceptors.dns() starts a separate lookup for every concurrent request to the same hostname when no cached records are available.

Requests arriving before the first lookup completes do not share its result. This causes bursts of DNS lookups on a cold cache and when cached records expire, even with a single shared dispatcher and DNS interceptor.

Reproduction

Standalone reproduction script:

'use strict'

const { test } = require('node:test')
const assert = require('node:assert/strict')
const { createServer } = require('node:http')
const { once } = require('node:events')
const { Agent, interceptors } = require('undici')

test('DNS interceptor should share concurrent lookups', { timeout: 60000 }, async (t) => {
  const server = createServer((req, res) => res.end('ok'))
  t.after(() => new Promise((resolve) => server.close(resolve)))

  server.listen(0, '127.0.0.1')
  await once(server, 'listening')

  const origin = `http://dns-cache-test.invalid:${server.address().port}`
  let lookupCount = 0

  const dispatcher = new Agent().compose(
    interceptors.dns({
      maxTTL: 60000,
      lookup(origin, options, callback) {
        lookupCount++

        // Simulate asynchronous DNS resolution without an external DNS server.
        setTimeout(() => {
          callback(null, [
            { address: '127.0.0.1', family: 4, ttl: 60000 }
          ])
        }, 30)
      }
    })
  )

  t.after(() => dispatcher.close())

  async function runBatch() {
    await Promise.all(Array.from({ length: 100 }, async () => {
      const { body } = await dispatcher.request({
        origin,
        path: '/',
        method: 'GET'
      })

      await body.text()
    }))
  }

  await runBatch()
  const coldCacheLookups = lookupCount

  await runBatch()
  const warmCacheLookups = lookupCount - coldCacheLookups

  console.log({ coldCacheLookups, warmCacheLookups })

  assert.equal(warmCacheLookups, 0, 'Completed DNS results should be cached')
  assert.equal(coldCacheLookups, 1, 'Concurrent requests should share one DNS lookup')
})

Save the script as test/repro-dns-lookup-dedup.js and run:

node --test test/repro-dns-lookup-dedup.js

Expected Behavior

Concurrent requests resolving the same hostname with compatible lookup options should share a single in-flight lookup.

The first batch should perform one lookup. The second batch should use the cached records and perform no additional lookups.

After a lookup fails, pending state should be cleared so subsequent requests can retry.

Actual Behavior

The first batch performs 100 separate lookups for the same hostname. The second batch performs no additional lookups, confirming that completed results are cached.

The assertion checking that concurrent requests share one lookup fails.

Environment

Environment used to validate the standalone reproduction:

  • OS: Linux x64
  • Node.js version: v24.19.0
  • undici version: 8.10.0

Additional context

Our application previously used Node.js http/https with cacheable-lookup, which shares pending resolution operations per hostname. We did not observe these lookup bursts with that setup.

Our custom lookup delegates to dns.lookup() using all: true and order: 'ipv4first'. The delayed lookup in the reproduction isolates the concurrency issue without depending on external DNS.

A possible solution would be to track pending lookups separately from cached records, populate the cache once, and let each waiting request independently select an address through the existing pick logic.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions