http2 bottleneck? #83
Comments
|
OK! Let's try to understand what is happening. Immediately I'd say you are not leveraging the multiplexed aspect properly (across users) and you are not evaluating the results yielded by your tests properly. A quick test show the following results (https://rest.ably.io/time x HTTP/2): without multiplexing: around ~30 TPS (which is reasonable given the ~30ms response time) with a single thread. So the hundred users seems useless or not configured properly. This need further investigation. You will excuse me, I only have limited time to investigate this case. Regards,
I suspect you voided multiplexing by inadvertence. And if your load testing tool access the response immediately, the effect will be negated. |
|
@Ousret I am not sure how are you exactly running the tests. |
|
Please don't come to the conclusion that I'm not using |
|
Please clone the repo https://github.com/sacOO7/locust-httpx-testing/tree/niquests at |
|
Good to see you are considering it as your driver. We'll make sure it fit your needs by answering your concerns.
Yes. But also highly tied around the synchronous limits brought by Requests.
No worries, I am just issuing an hypothesis that the load testing tool does not leverage Niquests as it should.
Very simple piece of code, see: import niquests
import time
if __name__ == "__main__":
before = time.time()
responses = []
with niquests.Session(multiplexed=True) as s: # turn me off/on
while time.time() - before < 60.:
responses.append(
s.get("https://rest.ably.io/time")
)
s.gather()
delay = time.time() - before
print(delay)
print(len(responses) / delay)
We will give it a shot as soon as possible. |
|
Sure, I will be waiting! If |
|
So, I took some time to start the investigations. Locust requires to do the following "1 req -> 1 resp" per user so that it can collect "in real time" the status and draw the main graph. So, here is what I get with
And finally using HTTP/2
(i) A firm 1,5k/req per second without abnormal bumps. I didn't try exceeding 50 users due to my poor CPU capabilities, reaching high temp. But you get the idea. So, now, Locust can't explore properly multiplexing, it wasn't made this way. I hope this help. Code for locust from __future__ import annotations
import logging
import time
from contextlib import contextmanager
from typing import Generator
from urllib.parse import urlparse, urlunparse
import niquests
from locust import User
from locust.clients import absolute_http_url_regexp
from niquests import Response, Request
from niquests.auth import HTTPBasicAuth
from niquests.exceptions import MissingSchema, InvalidSchema, InvalidURL, RequestException
import urllib3
logger = logging.getLogger(__name__)
from locust.exception import LocustError, ResponseError, CatchResponseError
# Configure everything there!
SHARED_SESSION: niquests.Session = niquests.Session(
multiplexed=True,
pool_connections=1,
pool_maxsize=100,
disable_http2=True,
disable_http3=True,
)
class LocustResponse(Response):
def raise_for_status(self):
if hasattr(self, "error") and self.error:
raise self.error
Response.raise_for_status(self)
class ResponseContextManager(LocustResponse):
"""
A Response class that also acts as a context manager that provides the ability to manually
control if an HTTP request should be marked as successful or a failure in Locust's statistics
This class is a subclass of :py:class:`Response <requests.Response>` with two additional
methods: :py:meth:`success <locust.clients.ResponseContextManager.success>` and
:py:meth:`failure <locust.clients.ResponseContextManager.failure>`.
"""
_manual_result: bool | Exception | None = None
_entered = False
def __init__(self, response, request_event, request_meta):
# copy data from response to this object
self.__dict__ = response.__dict__
self._request_event = request_event
self.request_meta = request_meta
def __enter__(self):
self._entered = True
return self
def __exit__(self, exc, value, traceback):
# if the user has already manually marked this response as failure or success
# we can ignore the default behaviour of letting the response code determine the outcome
if self._manual_result is not None:
if self._manual_result is True:
self.request_meta["exception"] = None
elif isinstance(self._manual_result, Exception):
self.request_meta["exception"] = self._manual_result
self._report_request()
return exc is None
if exc:
if isinstance(value, ResponseError):
self.request_meta["exception"] = value
self._report_request()
else:
# we want other unknown exceptions to be raised
return False
else:
# Since we use the Exception message when grouping failures, in order to not get
# multiple failure entries for different URLs for the same name argument, we need
# to temporarily override the response.url attribute
orig_url = self.url
self.url = self.request_meta["name"]
try:
self.raise_for_status()
except niquests.exceptions.RequestException as e:
while (
isinstance(
e,
(
niquests.exceptions.ConnectionError,
urllib3.exceptions.ProtocolError,
urllib3.exceptions.MaxRetryError,
urllib3.exceptions.NewConnectionError,
),
)
and e.__context__
# Not sure if the above exceptions can ever be the lowest level, but it is good to be sure
):
e = e.__context__
self.request_meta["exception"] = e
self._report_request()
self.url = orig_url
return True
def _report_request(self, exc=None):
self._request_event.fire(**self.request_meta)
def success(self):
"""
Report the response as successful
Example::
with self.client.get("/does/not/exist", catch_response=True) as response:
if response.status_code == 404:
response.success()
"""
if not self._entered:
raise LocustError(
"Tried to set status on a request that has not yet been made. Make sure you use a with-block, like this:\n\nwith self.client.request(..., catch_response=True) as response:\n response.success()"
)
self._manual_result = True
def failure(self, exc):
"""
Report the response as a failure.
if exc is anything other than a python exception (like a string) it will
be wrapped inside a CatchResponseError.
Example::
with self.client.get("/", catch_response=True) as response:
if response.content == b"":
response.failure("No data")
"""
if not self._entered:
raise LocustError(
"Tried to set status on a request that has not yet been made. Make sure you use a with-block, like this:\n\nwith self.client.request(..., catch_response=True) as response:\n response.failure(...)"
)
if not isinstance(exc, Exception):
exc = CatchResponseError(exc)
self._manual_result = exc
class HttpSession:
"""
Class for performing web requests and holding (session-) cookies between requests (in order
to be able to log in and out of websites). Each request is logged so that locust can display
statistics.
This is a slightly extended version of `python-request <http://python-requests.org>`_'s
:py:class:`requests.Session` class and mostly this class works exactly the same. However
the methods for making requests (get, post, delete, put, head, options, patch, request)
can now take a *url* argument that's only the path part of the URL, in which case the host
part of the URL will be prepended with the HttpSession.base_url which is normally inherited
from a User class' host attribute.
Each of the methods for making requests also takes two additional optional arguments which
are Locust specific and doesn't exist in python-requests. These are:
:param name: (optional) An argument that can be specified to use as label in Locust's statistics instead of the URL path.
This can be used to group different URL's that are requested into a single entry in Locust's statistics.
:param catch_response: (optional) Boolean argument that, if set, can be used to make a request return a context manager
to work as argument to a with statement. This will allow the request to be marked as a fail based on the content of the
response, even if the response code is ok (2xx). The opposite also works, one can use catch_response to catch a request
and then mark it as successful even if the response code was not (i.e 500 or 404).
"""
def __init__(self, base_url, request_event, user, *args, session: niquests.Session | None = None, **kwargs):
super().__init__(*args, **kwargs)
self.base_url = base_url
self.request_event = request_event
self.user = user
self.session = session
# User can group name, or use the group context manager to gather performance statistics under a specific name
# This is an alternative to passing in the "name" parameter to the requests function
self.request_name: str | None = None
# Check for basic authentication
parsed_url = urlparse(self.base_url)
if parsed_url.username and parsed_url.password:
netloc = parsed_url.hostname
if parsed_url.port:
netloc += ":%d" % parsed_url.port
# remove username and password from the base_url
self.base_url = urlunparse(
(parsed_url.scheme, netloc, parsed_url.path, parsed_url.params, parsed_url.query, parsed_url.fragment)
)
# configure requests to use basic auth
self.auth = HTTPBasicAuth(parsed_url.username, parsed_url.password)
def _build_url(self, path):
"""prepend url with hostname unless it's already an absolute URL"""
if absolute_http_url_regexp.match(path):
return path
else:
return f"{self.base_url}{path}"
@contextmanager
def rename_request(self, name: str) -> Generator[None, None, None]:
"""Group requests using the "with" keyword"""
self.request_name = name
try:
yield
finally:
self.request_name = None
def request(self, method, url, name=None, catch_response=False, context={}, **kwargs):
"""
Constructs and sends a :py:class:`requests.Request`.
Returns :py:class:`requests.Response` object.
:param method: method for the new :class:`Request` object.
:param url: URL for the new :class:`Request` object.
:param name: (optional) An argument that can be specified to use as label in Locust's statistics instead of the URL path.
This can be used to group different URL's that are requested into a single entry in Locust's statistics.
:param catch_response: (optional) Boolean argument that, if set, can be used to make a request return a context manager
to work as argument to a with statement. This will allow the request to be marked as a fail based on the content of the
response, even if the response code is ok (2xx). The opposite also works, one can use catch_response to catch a request
and then mark it as successful even if the response code was not (i.e 500 or 404).
:param params: (optional) Dictionary or bytes to be sent in the query string for the :class:`Request`.
:param data: (optional) Dictionary or bytes to send in the body of the :class:`Request`.
:param headers: (optional) Dictionary of HTTP Headers to send with the :class:`Request`.
:param cookies: (optional) Dict or CookieJar object to send with the :class:`Request`.
:param files: (optional) Dictionary of ``'filename': file-like-objects`` for multipart encoding upload.
:param auth: (optional) Auth tuple or callable to enable Basic/Digest/Custom HTTP Auth.
:param timeout: (optional) How long in seconds to wait for the server to send data before giving up, as a float,
or a (`connect timeout, read timeout <user/advanced.html#timeouts>`_) tuple.
:type timeout: float or tuple
:param allow_redirects: (optional) Set to True by default.
:type allow_redirects: bool
:param proxies: (optional) Dictionary mapping protocol to the URL of the proxy.
:param stream: (optional) whether to immediately download the response content. Defaults to ``False``.
:param verify: (optional) if ``True``, the SSL cert will be verified. A CA_BUNDLE path can also be provided.
:param cert: (optional) if String, path to ssl client cert file (.pem). If Tuple, ('cert', 'key') pair.
"""
# if group name has been set and no name parameter has been passed in; set the name parameter to group_name
if self.request_name and not name:
name = self.request_name
# prepend url with hostname unless it's already an absolute URL
url = self._build_url(url)
start_time = time.time()
start_perf_counter = time.perf_counter()
response = self._send_request_safe_mode(method, url, **kwargs)
response_time = (time.perf_counter() - start_perf_counter) * 1000
request_before_redirect = (response.history and response.history[0] or response).request
url = request_before_redirect.url
if not name:
name = request_before_redirect.path_url
if self.user:
context = {**self.user.context(), **context}
# store meta data that is used when reporting the request to locust's statistics
request_meta = {
"request_type": method,
"response_time": response_time,
"name": name,
"context": context,
"response": response,
"exception": None,
"start_time": start_time,
"url": url,
}
# get the length of the content, but if the argument stream is set to True, we take
# the size from the content-length header, in order to not trigger fetching of the body
if kwargs.get("stream", False):
request_meta["response_length"] = int(response.headers.get("content-length") or 0)
else:
request_meta["response_length"] = len(response.content or b"")
if catch_response:
return ResponseContextManager(response, request_event=self.request_event, request_meta=request_meta)
else:
with ResponseContextManager(response, request_event=self.request_event, request_meta=request_meta):
pass
return response
def _send_request_safe_mode(self, method, url, **kwargs):
"""
Send an HTTP request, and catch any exception that might occur due to connection problems.
Safe mode has been removed from requests 1.x.
"""
try:
return self.session.request(method, url, **kwargs)
except (MissingSchema, InvalidSchema, InvalidURL):
raise
except RequestException as e:
r = LocustResponse()
r.error = e
r.status_code = 0 # with this status_code, content returns None
r.request = Request(method, url).prepare()
return r
class NiquestsUser(User):
abstract = True
"""If abstract is True, the class is meant to be subclassed, and users will not choose this locust during a test"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.host is None:
raise LocustError(
"You must specify the base host. Either in the host attribute in the User class, or on the command line using the --host option."
)
self.client = HttpSession(
base_url=self.host,
request_event=self.environment.events.request,
user=self,
session=SHARED_SESSION,
)and patch the locustfile as follow from locust import task
from niquest_user import NiquestsUser
class NiquestsTestUser(NiquestsUser):
@task
def fetch_ably_time(self):
self.client.request("GET", "/time") |
|
Hi @Ousret thanks for the response! To make things easy, I have written a script -> https://github.com/sacOO7/locust-httpx-testing/blob/main/cli-script.py. Found bug for httpx using the same script, you can check discussion here -> encode/httpx#3100. You can replace usage of requests with the |
|
OK. I took a deeper look into the script. my results correlate with above. the only surprising thing is that httpx crash randomly with random stacktraces+deadlock rarely (conn drop, keyerror, conn reset, etc..) and tried to lower the thread count also to 25, without success, http2 is hard to maintain in a multi-threaded environment it seems. +1 to niquests then..! |
|
Hi @Ousret thanks for the response! Actually, |
I didn't get any crash using http 1.1 using a dummy local server (golang httpbin) crash http2 local and remote no matter what. http 1 crash
why not, but will do it later as I have to run it several times to collect the different crash.
Well, yes but actually no. If http2 is implemented with multiplexing in mind, it is worthy to be considered.
We'll make sure of it. |
|
Here are the traces you requested, for good measures, just upgraded httpx/httpcore to latest. Well, I consider that this thread answered your initial concerns. I am not going to dig further into httpx for obvious reasons. Others exceptions seems kind of hard to reach. I have no idea about the origin of the disfunctions. I am closing this as I don't think further things are awaited from my part. Regards, |
|
Thanks @Ousret. This is really helpful : ). I will open up a new issue if needed 👍 |
|
Update: We've landed some improvements over the performances. http/1
http/2
results:
with the given locust scenario. and still with smooth graphs. |
|
Ohkay, how did we exactly do it? Did we add some code recently for performance improvements? Good to see there is a room for improvements under load : ) |
|
Re: I knew in advance in early stage dev that some key spots needed to be optimized in both Niquests and urllib3.future. |
|
Also, try using server hosted on the cloud |
|
Okay, I just saw your latest commit. Maybe you will need something like |
worked fine without issues.
already the case. we've almost ran out of optimization that are "safe". |
|
Here is a little update on this topic. previously hit 1.5k req/s using http/2. now I can hit comfortably 1.9k req/s using http/2...
And that, in a pure synchronous loop. Without advanced multiplexing. No improvement for http/1 as we remain at a stable 1.8k req/s.
without locust, using a dead simple script (async + multiplexing) I am getting somewhere around 2.5k req/s with http/2. |
|
Thanks, good to hear we were able to optimise the library 🙂👍. |
|
Were you able to do some testing? I would gladly receive any feedback from your dev team. last time I wrote, we broke previous record, 1.9k using http2. now..!
how about 2.1k? still using http2. and look at the timings graph! still straight? isn't it? regards, |
|
Damn, great to hear on the progress made 👍 |
|
Great to see you like those improvements! I understand about the priorities.
we may expect a significant rise very soon. hopefully. |
niquestsis behaving more like Httpx not production ready under load encode/httpx#3100, it's still slower than requests. This is for 100 users withhttp2= True andmultiplexing= Trueagainsts requests
pool_connectionsandpool_maxsize, it just doesn't work via options. I had to set it explicitly via adapter.The text was updated successfully, but these errors were encountered: