srv_resolver: add 15s timeout to DNS lookups #19026

amaanq · 2025-10-07T19:34:14Z

Closes synapse.http.federation.srv_resolver.SrvResolver.resolve_service isn't able to "timeout" properly, and thus stalls federation #9774

Problem

The default timeout when looking up SRV records for the dns client is 60 seconds, which is quite long and can be problematic as that's the value of the default http client timeout, thus the error surfaced is the http error and not the underlying DNS error.

Solution

I've added a more aggressive timeout of 15 seconds total (with retries after 1, 3, 3, 3, and 5 seconds) for the SRV lookup. I've also re-labeled the `defer.TimeoutError, as that was done in the prior synapse PR, and makes the error more clear to users.

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Code style is correct (run the linters)

CLAassistant · 2025-10-07T19:34:22Z

All committers have signed the CLA.

synapse/http/federation/srv_resolver.py

MadLittleMods · 2025-10-07T23:03:16Z

synapse/http/federation/srv_resolver.py

        try:
            answers, _, _ = await make_deferred_yieldable(
-                self._dns_client.lookupService(service_name)
+                self._dns_client.lookupService(service_name, timeout=(1, 1, 2, 4, 2))


We should include all of the juicy context for why we're doing this in the comments. I've already written this out in #19026 (comment)

Done, thank you very much! Do you want me to add you as a co-author?

I appreciate the offer! It's normal to help out contributors and Element pays me so we can skip.

May be reasonable to add @ShadowJonathan as a co-author as we cribbed their TimeoutError change from matrix-org/synapse#9776

MadLittleMods · 2025-10-07T23:13:36Z

synapse/http/federation/srv_resolver.py

+                        # TODO: In the future, we could consider removing this extra
+                        # time if we don't see complaints. For comparison, The Windows


Suggested change

# TODO: In the future, we could consider removing this extra

# time if we don't see complaints. For comparison, The Windows

# TODO: In the future (after 2026-01-01), we could consider removing this extra

# time if we don't see complaints. For comparison, the Windows

My own comment but noticed one typo.

And I think it would be good to add a date for anyone stumbling upon this and wondering when/if we can make the change.

MadLittleMods · 2025-10-07T23:16:24Z

changelog.d/19026.misc

+Reduces the SRV DNS record lookup timeout to 15 seconds.
+This fixes issues when DNS lookups hang, as the default timeout of 60 seconds matches the timeout for the federation request itself,
+thus we see the HTTP request timeout and not the actual DNS error.


I don't know how line wrapping works with the Towncrier changelog entries but I never wrap them so that's what I'd go with.

MadLittleMods · 2025-10-07T23:21:16Z

synapse/http/federation/srv_resolver.py

                raise e
+        except defer.TimeoutError as e:
+            raise defer.TimeoutError(
+                f"Could not resolve DNS for SRV record {service_name!r} due to timeout (50s total)"


Suggested change

f"Could not resolve DNS for SRV record {service_name!r} due to timeout (50s total)"

f"Timed out while trying to resolve DNS for SRV record for {service_name!r} (timeout=15s)"

Does this sound better?

Only real nit is this previously said 50s total vs our new 15s timeout. Ideally, we'd have a constant to use here but I'm not sure that moving timeout=(1, 3, 3, 3, 5) to the top as a constant is better. The comments probably read better in place where it's used.

I'd say to make it a constant, then its easy to do the following:

Suggested change

f"Could not resolve DNS for SRV record {service_name!r} due to timeout (50s total)"

f"Timed out while trying to resolve DNS for SRV record for {service_name!r} (timeout={sum(LOOKUP_TIMEOUTS)}s)"

MadLittleMods · 2025-10-07T23:22:15Z

synapse/http/federation/srv_resolver.py

                return list(cache_entry)
            else:
                raise e
+        except defer.TimeoutError as e:


It would be nice to have a test that stressed this part of the code. Especially since I'm unsure if defer.TimeoutError is actually the exception type raised here.

Do you think you would be up for that? Probably involves reactor.advance(15 + 1) to advance time past the timeout. Otherwise, I can take a stab at it.

MadLittleMods · 2025-10-07T23:31:00Z

changelog.d/19026.misc

+Reduces the SRV DNS record lookup timeout to 15 seconds.
+This fixes issues when DNS lookups hang, as the default timeout of 60 seconds matches the timeout for the federation request itself,
+thus we see the HTTP request timeout and not the actual DNS error.


Suggested change

Reduces the SRV DNS record lookup timeout to 15 seconds.

This fixes issues when DNS lookups hang, as the default timeout of 60 seconds matches the timeout for the federation request itself,

thus we see the HTTP request timeout and not the actual DNS error.

Shorten DNS resolver timeout/retry sequence from 60s to 15s to ensure DNS failures are visible before federation HTTP request timeouts.

MadLittleMods · 2025-10-07T23:37:01Z

synapse/http/federation/srv_resolver.py

        try:
            answers, _, _ = await make_deferred_yieldable(
-                self._dns_client.lookupService(service_name)
+                self._dns_client.lookupService(service_name, timeout=(1, 1, 2, 4, 2))


I appreciate the offer! It's normal to help out contributors and Element pays me so we can skip.

May be reasonable to add @ShadowJonathan as a co-author as we cribbed their TimeoutError change from matrix-org/synapse#9776

ShadowJonathan

15 seconds may be short, but I think that if a server isn't responding inbetween those multi-second retries, its having other issues anyways.

LGTM

amaanq requested a review from a team as a code owner October 7, 2025 19:34

MadLittleMods requested changes Oct 7, 2025

View reviewed changes

synapse/http/federation/srv_resolver.py Show resolved Hide resolved

synapse/http/federation/srv_resolver.py Show resolved Hide resolved

synapse/http/federation/srv_resolver.py Outdated Show resolved Hide resolved

synapse/http/federation/srv_resolver.py Show resolved Hide resolved

MadLittleMods added the A-Federation label Oct 7, 2025

amaanq force-pushed the srv-timeout branch from 18ce3a7 to 0145b4e Compare October 7, 2025 23:01

MadLittleMods reviewed Oct 7, 2025

View reviewed changes

amaanq force-pushed the srv-timeout branch 2 times, most recently from 28b9258 to 5059cd5 Compare October 7, 2025 23:05

srv_resolver: add 15s timeout to DNS lookups

aaacc87

amaanq force-pushed the srv-timeout branch from 5059cd5 to aaacc87 Compare October 7, 2025 23:13

MadLittleMods reviewed Oct 7, 2025

View reviewed changes

ShadowJonathan approved these changes Oct 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

srv_resolver: add 15s timeout to DNS lookups #19026

srv_resolver: add 15s timeout to DNS lookups #19026

amaanq commented Oct 7, 2025 •

edited

Loading

Uh oh!

CLAassistant commented Oct 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

amaanq Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

ShadowJonathan Oct 9, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

MadLittleMods Oct 7, 2025

Uh oh!

ShadowJonathan left a comment

Uh oh!

Uh oh!

		# TODO: In the future, we could consider removing this extra
		# time if we don't see complaints. For comparison, The Windows

	f"Could not resolve DNS for SRV record {service_name!r} due to timeout (50s total)"
	f"Timed out while trying to resolve DNS for SRV record for {service_name!r} (timeout=15s)"

srv_resolver: add 15s timeout to DNS lookups #19026

Are you sure you want to change the base?

srv_resolver: add 15s timeout to DNS lookups #19026

Conversation

amaanq commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Pull Request Checklist

Uh oh!

CLAassistant commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShadowJonathan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amaanq commented Oct 7, 2025 •

edited

Loading

CLAassistant commented Oct 7, 2025 •

edited

Loading