Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Replaced GAIResolver with twisted.names.client for the default resolver #5053

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5053.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace blocking getaddrinfo hostname resolver with the non-blocking resolver from twisted.names.client.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Replace blocking getaddrinfo hostname resolver with the non-blocking resolver from twisted.names.client.
Add an option to replace the blocking `getaddrinfo` hostname resolver with the non-blocking resolver from `twisted.names.client`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(could call this a feature rather than a misc?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, feel free to add yourself as a contributor here, along the lines of the other entries in CHANGES.md.

6 changes: 6 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ pid_file: DATADIR/homeserver.pid
#
#cpu_affinity: 0xFFFFFFFF

# Whether to use the default system resolver (getaddrinfo) instead of the twisted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this is a generated file (the script to update it is scripts-dev/generate-sample-config). Please could you update the source for it (in synapse/config/server.py) and then run the script to update the sample config?

# asynchronous dns resolver. Using the async resolver can potentially improve
# performance, but may also behave different from getaddrinfo.
# Defaults to True. Uncomment to use the twisted resolver instead.
#use_getaddrinfo_for_dns: False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while you're changing the source, please could you add a line with just # between the text and the option, for consistency with the rest of the file.


# The path to the web client which will be served at /_matrix/client/
# if 'webclient' is configured under the 'listeners' configuration.
#
Expand Down
8 changes: 8 additions & 0 deletions synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import psutil
from daemonize import Daemonize

import twisted.names
from twisted.internet import defer, error, reactor
from twisted.protocols.tls import TLSMemoryBIOFactory

Expand Down Expand Up @@ -70,6 +71,7 @@ def start_worker_reactor(appname, config):
daemonize=config.worker_daemonize,
cpu_affinity=config.worker_cpu_affinity,
print_pidfile=config.print_pidfile,
use_getaddrinfo_for_dns=config.use_getaddrinfo_for_dns,
logger=logger,
)

Expand All @@ -82,6 +84,7 @@ def start_reactor(
daemonize,
cpu_affinity,
print_pidfile,
use_getaddrinfo_for_dns,
logger,
):
""" Run the reactor in the main process
Expand All @@ -96,6 +99,7 @@ def start_reactor(
pid_file (str): name of pid file to write to if daemonize is True
daemonize (bool): true to run the reactor in a background process
cpu_affinity (int|None): cpu affinity mask
use_getaddrinfo_for_dns (bool): use getaddrinfo instead of async resolver
print_pidfile (bool): whether to print the pid file, if daemonize is True
logger (logging.Logger): logger instance to pass to Daemonize
"""
Expand Down Expand Up @@ -127,6 +131,10 @@ def run():
change_resource_limit(soft_file_limit)
if gc_thresholds:
gc.set_threshold(*gc_thresholds)

if use_getaddrinfo_for_dns:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surely this condition is reversed?

reactor.installResolver(twisted.names.client)

reactor.run()

if daemonize:
Expand Down
1 change: 1 addition & 0 deletions synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ def start_generate_monthly_active_users():
daemonize=hs.config.daemonize,
cpu_affinity=hs.config.cpu_affinity,
print_pidfile=hs.config.print_pidfile,
use_getaddrinfo_for_dns=hs.config.use_getaddrinfo_for_dns,
logger=logger,
)

Expand Down
6 changes: 6 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ def read_config(self, config):
self.public_baseurl = config.get("public_baseurl")
self.cpu_affinity = config.get("cpu_affinity")

# Whether to use the getaddrinfo instead of the asynchronous DNS
# resolver. Disabling this can potentially improve performance.
self.use_getaddrinfo_for_dns = config.get(
"use_getaddrinfo_for_dns", True
)

# Whether to send federation traffic out in this process. This only
# applies to some federation traffic, and so shouldn't be used to
# "disable" federation
Expand Down