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

Add reactor to SynapseRequest and fix up types. #10868

Merged
merged 5 commits into from
Sep 24, 2021
Merged
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/10868.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Speed up responding with large JSON objects to requests.
4 changes: 2 additions & 2 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def __init__(self, canonical_json=False, extract_context=False):

def _send_response(
self,
request: Request,
request: SynapseRequest,
code: int,
response_object: Any,
):
Expand Down Expand Up @@ -629,7 +629,7 @@ def _encode_json_bytes(json_object: Any) -> Iterator[bytes]:


def respond_with_json(
request: Request,
request: SynapseRequest,
Copy link
Member

@clokep clokep Sep 21, 2021

Choose a reason for hiding this comment

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

This doesn't look like it needs a SynapseRequest? I'm guessing that's coming in a different PR? (This is true for a handful of methods changed...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes it is.

code: int,
json_object: Any,
send_cors: bool = False,
Expand Down
37 changes: 24 additions & 13 deletions synapse/http/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
import contextlib
import logging
import time
from typing import Optional, Tuple, Union
from typing import Generator, Optional, Tuple, Union

import attr
from zope.interface import implementer

from twisted.internet.interfaces import IAddress, IReactorTime
from twisted.python.failure import Failure
from twisted.web.http import HTTPChannel
from twisted.web.resource import IResource, Resource
from twisted.web.server import Request, Site

Expand Down Expand Up @@ -61,10 +62,18 @@ class SynapseRequest(Request):
logcontext: the log context for this request
"""

def __init__(self, channel, *args, max_request_body_size: int = 1024, **kw):
Request.__init__(self, channel, *args, **kw)
def __init__(
self,
channel: HTTPChannel,
site: "SynapseSite",
*args,
max_request_body_size: int = 1024,
**kw,
):
super().__init__(channel, *args, **kw)
self._max_request_body_size = max_request_body_size
self.site: SynapseSite = channel.site
self.synapse_site = site
self.reactor = site.reactor
self._channel = channel # this is used by the tests
self.start_time = 0.0

Expand Down Expand Up @@ -97,7 +106,7 @@ def __repr__(self) -> str:
self.get_method(),
self.get_redacted_uri(),
self.clientproto.decode("ascii", errors="replace"),
self.site.site_tag,
self.synapse_site.site_tag,
)

def handleContentChunk(self, data: bytes) -> None:
Expand Down Expand Up @@ -216,7 +225,7 @@ def render(self, resrc: Resource) -> None:
request=ContextRequest(
request_id=request_id,
ip_address=self.getClientIP(),
site_tag=self.site.site_tag,
site_tag=self.synapse_site.site_tag,
# The requester is going to be unknown at this point.
requester=None,
authenticated_entity=None,
Expand All @@ -228,7 +237,7 @@ def render(self, resrc: Resource) -> None:
)

# override the Server header which is set by twisted
self.setHeader("Server", self.site.server_version_string)
self.setHeader("Server", self.synapse_site.server_version_string)

with PreserveLoggingContext(self.logcontext):
# we start the request metrics timer here with an initial stab
Expand All @@ -247,7 +256,7 @@ def render(self, resrc: Resource) -> None:
requests_counter.labels(self.get_method(), self.request_metrics.name).inc()

@contextlib.contextmanager
def processing(self):
def processing(self) -> Generator[None, None, None]:
"""Record the fact that we are processing this request.

Returns a context manager; the correct way to use this is:
Expand Down Expand Up @@ -346,10 +355,10 @@ def _started_processing(self, servlet_name: str) -> None:
self.start_time, name=servlet_name, method=self.get_method()
)

self.site.access_logger.debug(
self.synapse_site.access_logger.debug(
"%s - %s - Received request: %s %s",
self.getClientIP(),
self.site.site_tag,
self.synapse_site.site_tag,
self.get_method(),
self.get_redacted_uri(),
)
Expand Down Expand Up @@ -388,13 +397,13 @@ def _finished_processing(self) -> None:
if authenticated_entity:
requester = f"{authenticated_entity}|{requester}"

self.site.access_logger.log(
self.synapse_site.access_logger.log(
log_level,
"%s - %s - {%s}"
" Processed request: %.3fsec/%.3fsec (%.3fsec, %.3fsec) (%.3fsec/%.3fsec/%d)"
' %sB %s "%s %s %s" "%s" [%d dbevts]',
self.getClientIP(),
self.site.site_tag,
self.synapse_site.site_tag,
requester,
processing_time,
response_send_time,
Expand Down Expand Up @@ -522,7 +531,7 @@ def __init__(
site_tag: str,
config: ListenerConfig,
resource: IResource,
server_version_string,
server_version_string: str,
max_request_body_size: int,
reactor: IReactorTime,
):
Expand All @@ -542,6 +551,7 @@ def __init__(
Site.__init__(self, resource, reactor=reactor)

self.site_tag = site_tag
self.reactor = reactor

assert config.http_options is not None
proxied = config.http_options.x_forwarded
Expand All @@ -550,6 +560,7 @@ def __init__(
def request_factory(channel, queued: bool) -> Request:
return request_class(
channel,
self,
max_request_body_size=max_request_body_size,
queued=queued,
)
Expand Down
9 changes: 4 additions & 5 deletions synapse/rest/key/v2/remote_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@

from signedjson.sign import sign_json

from twisted.web.server import Request

from synapse.api.errors import Codes, SynapseError
from synapse.crypto.keyring import ServerKeyFetcher
from synapse.http.server import DirectServeJsonResource, respond_with_json
from synapse.http.servlet import parse_integer, parse_json_object_from_request
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict
from synapse.util import json_decoder
from synapse.util.async_helpers import yieldable_gather_results
Expand Down Expand Up @@ -100,7 +99,7 @@ def __init__(self, hs: "HomeServer"):
self.federation_domain_whitelist = hs.config.federation_domain_whitelist
self.config = hs.config

async def _async_render_GET(self, request: Request) -> None:
async def _async_render_GET(self, request: SynapseRequest) -> None:
assert request.postpath is not None
if len(request.postpath) == 1:
(server,) = request.postpath
Expand All @@ -117,7 +116,7 @@ async def _async_render_GET(self, request: Request) -> None:

await self.query_keys(request, query, query_remote_on_cache_miss=True)

async def _async_render_POST(self, request: Request) -> None:
async def _async_render_POST(self, request: SynapseRequest) -> None:
content = parse_json_object_from_request(request)

query = content["server_keys"]
Expand All @@ -126,7 +125,7 @@ async def _async_render_POST(self, request: Request) -> None:

async def query_keys(
self,
request: Request,
request: SynapseRequest,
squahtx marked this conversation as resolved.
Show resolved Hide resolved
query: JsonDict,
query_remote_on_cache_miss: bool = False,
) -> None:
Expand Down
7 changes: 4 additions & 3 deletions synapse/rest/media/v1/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

from synapse.api.errors import Codes, SynapseError, cs_error
from synapse.http.server import finish_request, respond_with_json
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable
from synapse.util.stringutils import is_ascii

Expand Down Expand Up @@ -74,7 +75,7 @@ def parse_media_id(request: Request) -> Tuple[str, str, Optional[str]]:
)


def respond_404(request: Request) -> None:
def respond_404(request: SynapseRequest) -> None:
respond_with_json(
request,
404,
Expand All @@ -84,7 +85,7 @@ def respond_404(request: Request) -> None:


async def respond_with_file(
request: Request,
request: SynapseRequest,
media_type: str,
file_path: str,
file_size: Optional[int] = None,
Expand Down Expand Up @@ -221,7 +222,7 @@ def _can_encode_filename_as_token(x: str) -> bool:


async def respond_with_responder(
request: Request,
request: SynapseRequest,
responder: "Optional[Responder]",
media_type: str,
file_size: Optional[int],
Expand Down
4 changes: 1 addition & 3 deletions synapse/rest/media/v1/config_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

from typing import TYPE_CHECKING

from twisted.web.server import Request

from synapse.http.server import DirectServeJsonResource, respond_with_json
from synapse.http.site import SynapseRequest

Expand All @@ -39,5 +37,5 @@ async def _async_render_GET(self, request: SynapseRequest) -> None:
await self.auth.get_user_by_req(request)
respond_with_json(request, 200, self.limits_dict, send_cors=True)

async def _async_render_OPTIONS(self, request: Request) -> None:
async def _async_render_OPTIONS(self, request: SynapseRequest) -> None:
respond_with_json(request, 200, {}, send_cors=True)
5 changes: 2 additions & 3 deletions synapse/rest/media/v1/download_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
import logging
from typing import TYPE_CHECKING

from twisted.web.server import Request

from synapse.http.server import DirectServeJsonResource, set_cors_headers
from synapse.http.servlet import parse_boolean
from synapse.http.site import SynapseRequest

from ._base import parse_media_id, respond_404

Expand All @@ -37,7 +36,7 @@ def __init__(self, hs: "HomeServer", media_repo: "MediaRepository"):
self.media_repo = media_repo
self.server_name = hs.hostname

async def _async_render_GET(self, request: Request) -> None:
async def _async_render_GET(self, request: SynapseRequest) -> None:
set_cors_headers(request)
request.setHeader(
b"Content-Security-Policy",
Expand Down
10 changes: 7 additions & 3 deletions synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import twisted.web.http
from twisted.internet.defer import Deferred
from twisted.web.resource import Resource
from twisted.web.server import Request

from synapse.api.errors import (
FederationDeniedError,
Expand All @@ -34,6 +33,7 @@
)
from synapse.config._base import ConfigError
from synapse.config.repository import ThumbnailRequirement
from synapse.http.site import SynapseRequest
from synapse.logging.context import defer_to_thread
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.types import UserID
Expand Down Expand Up @@ -187,7 +187,7 @@ async def create_content(
return "mxc://%s/%s" % (self.server_name, media_id)

async def get_local_media(
self, request: Request, media_id: str, name: Optional[str]
self, request: SynapseRequest, media_id: str, name: Optional[str]
) -> None:
"""Responds to requests for local media, if exists, or returns 404.

Expand Down Expand Up @@ -221,7 +221,11 @@ async def get_local_media(
)

async def get_remote_media(
self, request: Request, server_name: str, media_id: str, name: Optional[str]
self,
request: SynapseRequest,
server_name: str,
media_id: str,
name: Optional[str],
) -> None:
"""Respond to requests for remote media.

Expand Down
3 changes: 1 addition & 2 deletions synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

from twisted.internet.defer import Deferred
from twisted.internet.error import DNSLookupError
from twisted.web.server import Request

from synapse.api.errors import Codes, SynapseError
from synapse.http.client import SimpleHttpClient
Expand Down Expand Up @@ -168,7 +167,7 @@ def __init__(
self._start_expire_url_cache_data, 10 * 1000
)

async def _async_render_OPTIONS(self, request: Request) -> None:
async def _async_render_OPTIONS(self, request: SynapseRequest) -> None:
request.setHeader(b"Allow", b"OPTIONS, GET")
respond_with_json(request, 200, {}, send_cors=True)

Expand Down
15 changes: 7 additions & 8 deletions synapse/rest/media/v1/thumbnail_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple

from twisted.web.server import Request

from synapse.api.errors import SynapseError
from synapse.http.server import DirectServeJsonResource, set_cors_headers
from synapse.http.servlet import parse_integer, parse_string
from synapse.http.site import SynapseRequest
from synapse.rest.media.v1.media_storage import MediaStorage

from ._base import (
Expand Down Expand Up @@ -57,7 +56,7 @@ def __init__(
self.dynamic_thumbnails = hs.config.dynamic_thumbnails
self.server_name = hs.hostname

async def _async_render_GET(self, request: Request) -> None:
async def _async_render_GET(self, request: SynapseRequest) -> None:
set_cors_headers(request)
server_name, media_id, _ = parse_media_id(request)
width = parse_integer(request, "width", required=True)
Expand Down Expand Up @@ -88,7 +87,7 @@ async def _async_render_GET(self, request: Request) -> None:

async def _respond_local_thumbnail(
self,
request: Request,
request: SynapseRequest,
media_id: str,
width: int,
height: int,
Expand Down Expand Up @@ -121,7 +120,7 @@ async def _respond_local_thumbnail(

async def _select_or_generate_local_thumbnail(
self,
request: Request,
request: SynapseRequest,
media_id: str,
desired_width: int,
desired_height: int,
Expand Down Expand Up @@ -186,7 +185,7 @@ async def _select_or_generate_local_thumbnail(

async def _select_or_generate_remote_thumbnail(
self,
request: Request,
request: SynapseRequest,
server_name: str,
media_id: str,
desired_width: int,
Expand Down Expand Up @@ -249,7 +248,7 @@ async def _select_or_generate_remote_thumbnail(

async def _respond_remote_thumbnail(
self,
request: Request,
request: SynapseRequest,
server_name: str,
media_id: str,
width: int,
Expand Down Expand Up @@ -280,7 +279,7 @@ async def _respond_remote_thumbnail(

async def _select_and_respond_with_thumbnail(
self,
request: Request,
request: SynapseRequest,
desired_width: int,
desired_height: int,
desired_method: str,
Expand Down
Loading