Compare commits

...

2 Commits

Author SHA1 Message Date
Rebecca Law
eb808112d5 Add a canary app for testing 2021-01-26 13:47:55 +00:00
Rebecca Law
ab92618250 The delivery workers use a lot of CPU, we could find out where they are
using lots of CPU by tracing the application to gather some data

Alternatively we could take a stab in the dark, which is what this
commit is doing.

I have the hypothesis that we are not re-using TCP connections using
HTTP keepalive

Refer to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive

This means we are renegotiating the TLS connection every time we want to
send an SMS.  When we are sending lots of SMS messages then this will do
a lot of crypto handshaking which is expensive (in terms of CPU)

ie send_sms calls request() which creates a new tcp connection and a
new TLS handshake

When you use request.Session() to create a session it uses urllib3's
connection pooling, I've arbitrarily chosen 32 connections per pool
(with a default number of pools = 10)

ie init_app creates a session which has underlying connection pools
send_sms claims a connection from the pool and uses it to create or
re-use an existing TLS connection

Sessions are usually not great because they share data like cookies, but
when calling an API this is fine, or at least it is probably worth
canarying

Another way of re-using connections is by running HAProxy or similar as
a side-car proxy, which proxies to the API. send_sms would make a
local TCP connection to HAProxy which proxies to the MMG or Firetext
API via TLS, adding the Connection: keep-alive header

This command can be used to see how many TLS handshakes your computer is
doing, with some false positives:

tcpdump -n "tcp port 443 and (tcp[((tcp[12] & 0xf0) >> 2)] = 0x16)"

(alternatively we could just instrument the code /shrug)

Signed-off-by: toby lorne <toby@toby.codes>
2021-01-26 13:47:55 +00:00
5 changed files with 30 additions and 6 deletions

View File

@@ -62,6 +62,7 @@ class AwsSesClient(EmailClient):
# before-call, after-call, after-call-error, request-created, response-received
self._client.meta.events.register('request-created.ses.SendEmail', self.ses_request_created_hook)
self._client.meta.events.register('response-received.ses.SendEmail', self.ses_response_received_hook)
self._client.meta.events.register('before-call.ses', self.ses_inject_connection_header)
def ses_request_created_hook(self, **kwargs):
# request created may be called multiple times if the request auto-retries. We want to count all these as the
@@ -77,6 +78,11 @@ class AwsSesClient(EmailClient):
def get_name(self):
return self.name
def ses_inject_connection_header(self, params, **kwargs):
# keep underlying TLS connection open, so we do not spend lots of CPU
# and network time renegotiating TLS
params['headers']['Connection'] = 'Keep-Alive'
def send_email(self,
source,
to_addresses,
@@ -148,3 +154,4 @@ def punycode_encode_email(email_address):
# only the hostname should ever be punycode encoded.
local, hostname = email_address.split('@')
return '{}@{}'.format(local, hostname.encode('idna').decode('utf-8'))

View File

@@ -2,7 +2,8 @@ import json
import logging
from time import monotonic
from requests import request, RequestException
from requests import request, RequestException, Session
from requests.adapters import HTTPAdapter
from app.clients.sms import (SmsClient, SmsClientResponseException)
@@ -69,6 +70,9 @@ class FiretextClient(SmsClient):
self.name = 'firetext'
self.url = current_app.config.get('FIRETEXT_URL')
self.statsd_client = statsd_client
# this uses urllib3 under the hood to create a connection pool
self.session = Session()
self.session.mount('https://', HTTPAdapter(pool_maxsize=32))
def get_name(self):
return self.name
@@ -103,8 +107,7 @@ class FiretextClient(SmsClient):
response = None
start_time = monotonic()
try:
response = request(
"POST",
response = self.session.post(
self.url,
data=data,
timeout=60

View File

@@ -1,6 +1,8 @@
import json
from time import monotonic
from requests import (request, RequestException)
from requests import (request, RequestException, Session)
from requests.adapters import HTTPAdapter
from app.clients.sms import (SmsClient, SmsClientResponseException)
mmg_response_map = {
@@ -75,6 +77,9 @@ class MMGClient(SmsClient):
self.name = 'mmg'
self.statsd_client = statsd_client
self.mmg_url = current_app.config.get('MMG_URL')
# this uses urllib3 under the hood to create a connection pool
self.session = Session()
self.session.mount('https://', HTTPAdapter(pool_maxsize=32))
def record_outcome(self, success, response):
status_code = response.status_code if response else 503
@@ -108,8 +113,8 @@ class MMGClient(SmsClient):
response = None
start_time = monotonic()
try:
response = request(
"POST",
response = self.session.post(
self.mmg_url,
data=json.dumps(data),
headers={

View File

@@ -53,6 +53,11 @@
'notify-delivery-worker-jobs': {},
'notify-delivery-worker-research': {},
'notify-delivery-worker-sender': {'disk_quota': '2G', 'memory': '4G'},
'notify-delivery-worker-sender-canary': {'disk_quota': '2G', 'memory': '4G', 'instances': {
'preview': 0,
'staging': 1,
'production': 0
},},
'notify-delivery-worker-periodic': {},
'notify-delivery-worker-reporting': {
'additional_env_vars': {

View File

@@ -24,6 +24,10 @@ case $NOTIFY_APP_NAME in
exec scripts/run_multi_worker_app_paas.sh celery multi start 3 -c 10 -A run_celery.notify_celery --loglevel=INFO \
--logfile=/dev/null --pidfile=/tmp/celery%N.pid -Q send-sms-tasks,send-email-tasks
;;
delivery-worker-sender-canary)
exec scripts/run_multi_worker_app_paas.sh celery multi start 3 -c 10 -A run_celery.notify_celery --loglevel=INFO \
--logfile=/dev/null --pidfile=/tmp/celery%N.pid -Q send-sms-tasks,send-email-tasks
;;
delivery-worker-periodic)
exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=2 \
-Q periodic-tasks 2> /dev/null