Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serve /content/ routes with X-Sendfile #373

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

HebaruSan
Copy link
Contributor

@HebaruSan HebaruSan commented Jun 23, 2021

Background

The /download/ route handler tries to redirect mod downloads to the CDN here (_cfg("cdn-domain") is https://spacedock.info/content/):

protocol = _cfg("protocol")
cdn_domain = _cfg("cdn-domain")
if protocol and cdn_domain:
return redirect(protocol + '://' + cdn_domain + '/' + mod_version.download_path, code=302)

The idea is that the https://spacedock.info/content/ URLs should be handled by a fast server with access to the storage, what we've been calling AW (Apache Web Server).

Problem

CDN handling of downloads is disabled in the server config for reasons that nobody remembers (and turning it on temporarily produced unexplained timeouts and 502 statuses):

image

So the /content/ request comes back to the backend here:

@anonymous.route("/content/<path:path>")
def content(path: str) -> werkzeug.wrappers.Response:
storage = _cfg('storage')
if not storage or not os.path.isfile(os.path.join(storage, path)):
abort(404)
return send_from_directory(storage + "/", path)

And the gunicorn backend is handing out big files, as not intended.

But there is yet another better method for sending files which SD already supports: Apache's XSendFile module:

if _cfg("use-x-accel") == 'nginx':
response = make_response("")
response.headers['Content-Type'] = 'application/zip'
response.headers['Content-Disposition'] = 'attachment; filename=' + \
os.path.basename(mod_version.download_path)
response.headers['X-Accel-Redirect'] = '/internal/' + mod_version.download_path
if _cfg("use-x-accel") == 'apache':
response = make_response("")
response.headers['Content-Type'] = 'application/zip'
response.headers['Content-Disposition'] = 'attachment; filename=' + \
os.path.basename(mod_version.download_path)
response.headers['X-Sendfile'] = os.path.join(storage, mod_version.download_path)

This only happens for /download/ routes, if you set _cfg('use-x-accel') to nginx or apache and don't set _cfg("cdn-domain").

Changes

Now if you set _cfg('use-x-accel') it will also be used for /content/ routes. This should make them work better because the files will be sent by Apache instead of by gunicorn.

@DasSkelett
Copy link
Member

The idea is that the https://spacedock.info/content/ URLs should be handled by a fast server with access to the storage. I think that's Apache Traffic Server?

Nope, that one's the Apache Web Server (AW), which is inside the production container and has access to the storage mount. ATS only handles caching, some headers, and forwarding all web requests into the right containers depending on the accessed domain.

@DasSkelett
Copy link
Member

Works in my dev env after mounting the storage into the frontend container and setting use-x-accel=nginx (but leaving cdn-domain configured this time).
I'm also making nginx add a via header to make it clear whether it hits the /internal path via X-SendFile or not.

image

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Maybe we won't actually need it if we can get serving files from AW to work, but it's good to have a fallback, and simpler ways to test production-like setups locally.

@DasSkelett DasSkelett merged commit b88eee1 into KSP-SpaceDock:alpha Jun 23, 2021
@HebaruSan HebaruSan deleted the fix/xsendfile-content branch June 23, 2021 22:04
This was referenced Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants