From d20f4ad9353c7ddaaefd01e4f0b0d1fe563512cf Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Mon, 12 Aug 2024 20:01:00 -0300 Subject: [PATCH 1/5] feat: Use X-Accel-Redirect to improve file download speed Instead of making FastAPI handle file download, which has serious performance issues on big files [1], this change uses nginx's `X-Accel` feature to delegate single-file downloads to nginx. Partial fix for #1079, as it solves the CPU usage issue for single-file downloads. [1] https://github.com/fastapi/fastapi/discussions/6050 --- backend/config/__init__.py | 3 ++- backend/endpoints/rom.py | 22 +++++++++++++++++----- docker/nginx/default.conf | 6 ++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/backend/config/__init__.py b/backend/config/__init__.py index af698d32e..0293bdcba 100644 --- a/backend/config/__init__.py +++ b/backend/config/__init__.py @@ -13,7 +13,8 @@ GUNICORN_WORKERS: Final = int(os.environ.get("GUNICORN_WORKERS", 2)) # PATHS ROMM_BASE_PATH: Final = os.environ.get("ROMM_BASE_PATH", "/romm") -LIBRARY_BASE_PATH: Final = f"{ROMM_BASE_PATH}/library" +LIBRARY_PATH_SUFFIX: Final = "/library" +LIBRARY_BASE_PATH: Final = f"{ROMM_BASE_PATH}{LIBRARY_PATH_SUFFIX}" RESOURCES_BASE_PATH: Final = f"{ROMM_BASE_PATH}/resources" ASSETS_BASE_PATH: Final = f"{ROMM_BASE_PATH}/assets" FRONTEND_RESOURCES_PATH: Final = "/assets/romm/resources" diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index e69deab9e..d4868d8a0 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -9,6 +9,7 @@ from anyio import Path, open_file from config import ( DISABLE_DOWNLOAD_ENDPOINT_AUTH, LIBRARY_BASE_PATH, + LIBRARY_PATH_SUFFIX, RESOURCES_BASE_PATH, ) from decorators.auth import protected_route @@ -22,7 +23,7 @@ from endpoints.responses.rom import ( ) from exceptions.endpoint_exceptions import RomNotFoundInDatabaseException from exceptions.fs_exceptions import RomAlreadyExistsException -from fastapi import File, HTTPException, Query, Request, UploadFile, status +from fastapi import File, HTTPException, Query, Request, Response, UploadFile, status from fastapi.responses import FileResponse from handler.database import db_platform_handler, db_rom_handler from handler.filesystem import fs_resource_handler, fs_rom_handler @@ -210,17 +211,28 @@ async def get_rom_content( if not rom: raise RomNotFoundInDatabaseException(id) - rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" files_to_download = files or [r["filename"] for r in rom.files] if not rom.multi: - return FileResponse(path=rom_path, filename=rom.file_name) + return Response( + headers={ + "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', + "Content-Type": "application/octet-stream", + "X-Accel-Redirect": f"{LIBRARY_PATH_SUFFIX}/{rom.full_path}", + }, + ) if len(files_to_download) == 1: - return FileResponse( - path=f"{rom_path}/{files_to_download[0]}", filename=files_to_download[0] + return Response( + headers={ + "Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"', + "Content-Type": "application/octet-stream", + "X-Accel-Redirect": f"{LIBRARY_PATH_SUFFIX}/{rom.full_path}/{files_to_download[0]}", + }, ) + rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" + # Builds a generator of tuples for each member file async def local_files() -> AsyncIterator[AsyncMemberFile]: async def contents(filename: str) -> AsyncIterator[bytes]: diff --git a/docker/nginx/default.conf b/docker/nginx/default.conf index 5565cad7b..a93592c08 100644 --- a/docker/nginx/default.conf +++ b/docker/nginx/default.conf @@ -87,5 +87,11 @@ http { proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; } + + # Library files + location /library { + internal; + alias /romm/library; + } } } From 9281760975594653446ffd9f62ea82b059643419 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Tue, 13 Aug 2024 00:31:48 -0400 Subject: [PATCH 2/5] merge gzip changes into branch --- backend/config/__init__.py | 3 +-- backend/endpoints/rom.py | 31 ++++++++++++++++++++++--------- backend/main.py | 4 ---- docker/nginx/default.conf | 16 ++++++++++++++-- 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/backend/config/__init__.py b/backend/config/__init__.py index 0293bdcba..af698d32e 100644 --- a/backend/config/__init__.py +++ b/backend/config/__init__.py @@ -13,8 +13,7 @@ GUNICORN_WORKERS: Final = int(os.environ.get("GUNICORN_WORKERS", 2)) # PATHS ROMM_BASE_PATH: Final = os.environ.get("ROMM_BASE_PATH", "/romm") -LIBRARY_PATH_SUFFIX: Final = "/library" -LIBRARY_BASE_PATH: Final = f"{ROMM_BASE_PATH}{LIBRARY_PATH_SUFFIX}" +LIBRARY_BASE_PATH: Final = f"{ROMM_BASE_PATH}/library" RESOURCES_BASE_PATH: Final = f"{ROMM_BASE_PATH}/resources" ASSETS_BASE_PATH: Final = f"{ROMM_BASE_PATH}/assets" FRONTEND_RESOURCES_PATH: Final = "/assets/romm/resources" diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index d4868d8a0..c0a7dd72b 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -9,7 +9,6 @@ from anyio import Path, open_file from config import ( DISABLE_DOWNLOAD_ENDPOINT_AUTH, LIBRARY_BASE_PATH, - LIBRARY_PATH_SUFFIX, RESOURCES_BASE_PATH, ) from decorators.auth import protected_route @@ -23,7 +22,7 @@ from endpoints.responses.rom import ( ) from exceptions.endpoint_exceptions import RomNotFoundInDatabaseException from exceptions.fs_exceptions import RomAlreadyExistsException -from fastapi import File, HTTPException, Query, Request, Response, UploadFile, status +from fastapi import File, HTTPException, Query, Request, UploadFile, status from fastapi.responses import FileResponse from handler.database import db_platform_handler, db_rom_handler from handler.filesystem import fs_resource_handler, fs_rom_handler @@ -174,8 +173,19 @@ def head_rom_content(request: Request, id: int, file_name: str): rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" + if not rom.multi: + return FileResponse( + path=rom_path, + filename=rom.file_name, + headers={ + "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', + "Content-Type": "application/octet-stream", + "Content-Length": str(rom.file_size_bytes), + }, + ) + return FileResponse( - path=rom_path if not rom.multi else f'{rom_path}/{rom.files[0]["filename"]}', + path=f'{rom_path}/{rom.files[0]["filename"]}', filename=file_name, headers={ "Content-Disposition": f'attachment; filename="{quote(rom.name)}.zip"', @@ -211,28 +221,31 @@ async def get_rom_content( if not rom: raise RomNotFoundInDatabaseException(id) + rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" files_to_download = files or [r["filename"] for r in rom.files] if not rom.multi: - return Response( + return FileResponse( + path=rom_path, + filename=rom.file_name, headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', "Content-Type": "application/octet-stream", - "X-Accel-Redirect": f"{LIBRARY_PATH_SUFFIX}/{rom.full_path}", + "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) if len(files_to_download) == 1: - return Response( + return FileResponse( + path=f"{rom_path}/{files_to_download[0]}", + filename=files_to_download[0], headers={ "Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"', "Content-Type": "application/octet-stream", - "X-Accel-Redirect": f"{LIBRARY_PATH_SUFFIX}/{rom.full_path}/{files_to_download[0]}", + "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_download[0]}", }, ) - rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" - # Builds a generator of tuples for each member file async def local_files() -> AsyncIterator[AsyncMemberFile]: async def contents(filename: str) -> AsyncIterator[bytes]: diff --git a/backend/main.py b/backend/main.py index 8f3eb38b1..c810f870d 100644 --- a/backend/main.py +++ b/backend/main.py @@ -32,7 +32,6 @@ from endpoints import ( ) from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware -from fastapi.middleware.gzip import GZipMiddleware from handler.auth.base_handler import ALGORITHM from handler.auth.hybrid_auth import HybridAuthBackend from handler.auth.middleware import CustomCSRFMiddleware, SessionMiddleware @@ -68,9 +67,6 @@ if not IS_PYTEST_RUN and not DISABLE_CSRF_PROTECTION: exempt_urls=[re.compile(r"^/token.*"), re.compile(r"^/ws")], ) -# Enable GZip compression for responses -app.add_middleware(GZipMiddleware, minimum_size=1024) - # Handles both basic and oauth authentication app.add_middleware( AuthenticationMiddleware, diff --git a/docker/nginx/default.conf b/docker/nginx/default.conf index a93592c08..87896c31f 100644 --- a/docker/nginx/default.conf +++ b/docker/nginx/default.conf @@ -14,9 +14,14 @@ http { scgi_temp_path /tmp/scgi; sendfile on; + client_body_buffer_size 128k; client_max_body_size 0; + client_header_buffer_size 1k; + large_client_header_buffers 4 16k; + send_timeout 60s; + keepalive_timeout 65s; tcp_nopush on; - # types_hash_max_size 2048; + tcp_nodelay on; include /etc/nginx/mime.types; default_type application/octet-stream; @@ -41,6 +46,13 @@ http { error_log /dev/stderr; gzip on; + gzip_proxied any; + gzip_vary on; + gzip_comp_level 6; + gzip_buffers 16 8k; + gzip_min_length 1024; + gzip_http_version 1.1; + gzip_types text/plain text/css application/json application/javascript text/xml application/xml application/xml+rss text/javascript; # include /etc/nginx/conf.d/*.conf; # include /etc/nginx/sites-enabled/*; @@ -88,7 +100,7 @@ http { proxy_set_header Connection "upgrade"; } - # Library files + # Internally redirect download requests location /library { internal; alias /romm/library; From 0eb5bbad6d2cfdba2e5acc4533f9e9949354ed19 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 14 Aug 2024 18:39:49 -0400 Subject: [PATCH 3/5] add some missing headers --- backend/endpoints/rom.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index c0a7dd72b..af94117d8 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -29,7 +29,7 @@ from handler.filesystem import fs_resource_handler, fs_rom_handler from handler.filesystem.base_handler import CoverSize from handler.metadata import meta_igdb_handler, meta_moby_handler from logger.logger import log -from stream_zip import NO_COMPRESSION_32, ZIP_AUTO, AsyncMemberFile, async_stream_zip +from stream_zip import NO_COMPRESSION_64, ZIP_AUTO, AsyncMemberFile, async_stream_zip from utils.router import APIRouter router = APIRouter() @@ -231,6 +231,7 @@ async def get_rom_content( headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', "Content-Type": "application/octet-stream", + "Content-Length": str(rom.file_size_bytes), "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) @@ -242,6 +243,7 @@ async def get_rom_content( headers={ "Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"', "Content-Type": "application/octet-stream", + "Content-Length": str(rom.file_size_bytes), "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_download[0]}", }, ) @@ -277,7 +279,7 @@ async def get_rom_content( f"{file_name}.m3u", now, S_IFREG | 0o600, - NO_COMPRESSION_32, + NO_COMPRESSION_64, m3u_file(), ) @@ -288,7 +290,8 @@ async def get_rom_content( zipped_chunks, media_type="application/zip", headers={ - "Content-Disposition": f'attachment; filename="{quote(file_name)}.zip"' + "Content-Disposition": f'attachment; filename="{quote(file_name)}.zip"', + "Content-Type": "application/zip", }, emit_body={"id": rom.id}, ) From fed465a64d7caa4ce039864964b921432968d923 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Thu, 15 Aug 2024 23:27:01 -0400 Subject: [PATCH 4/5] use functions directly on file response --- backend/endpoints/rom.py | 53 +++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index af94117d8..2f6de260e 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -23,7 +23,7 @@ from endpoints.responses.rom import ( from exceptions.endpoint_exceptions import RomNotFoundInDatabaseException from exceptions.fs_exceptions import RomAlreadyExistsException from fastapi import File, HTTPException, Query, Request, UploadFile, status -from fastapi.responses import FileResponse +from fastapi.responses import Response from handler.database import db_platform_handler, db_rom_handler from handler.filesystem import fs_resource_handler, fs_rom_handler from handler.filesystem.base_handler import CoverSize @@ -154,7 +154,12 @@ def get_rom(request: Request, id: int) -> DetailedRomSchema: "/roms/{id}/content/{file_name}", [] if DISABLE_DOWNLOAD_ENDPOINT_AUTH else ["roms.read"], ) -def head_rom_content(request: Request, id: int, file_name: str): +async def head_rom_content( + request: Request, + id: int, + file_name: str, + files: Annotated[list[str] | None, Query()] = None, +): """Head rom content endpoint Args: @@ -172,25 +177,33 @@ def head_rom_content(request: Request, id: int, file_name: str): raise RomNotFoundInDatabaseException(id) rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" + files_to_check = files or [r["filename"] for r in rom.files] if not rom.multi: - return FileResponse( - path=rom_path, - filename=rom.file_name, + return Response( + media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', - "Content-Type": "application/octet-stream", "Content-Length": str(rom.file_size_bytes), + "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) - return FileResponse( - path=f'{rom_path}/{rom.files[0]["filename"]}', - filename=file_name, + if len(files_to_check) == 1: + file_path = f"{rom_path}/{files_to_check[0]}" + return Response( + media_type="application/octet-stream", + headers={ + "Content-Disposition": f'attachment; filename="{quote(files_to_check[0])}"', + "Content-Length": str((await Path(file_path).stat()).st_size), + "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_check[0]}", + }, + ) + + return Response( + media_type="application/zip", headers={ - "Content-Disposition": f'attachment; filename="{quote(rom.name)}.zip"', - "Content-Type": "application/zip", - "Content-Length": str(rom.file_size_bytes), + "Content-Disposition": f'attachment; filename="{quote(file_name)}.zip"', }, ) @@ -225,25 +238,22 @@ async def get_rom_content( files_to_download = files or [r["filename"] for r in rom.files] if not rom.multi: - return FileResponse( - path=rom_path, - filename=rom.file_name, + return Response( + media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', - "Content-Type": "application/octet-stream", "Content-Length": str(rom.file_size_bytes), "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) if len(files_to_download) == 1: - return FileResponse( - path=f"{rom_path}/{files_to_download[0]}", - filename=files_to_download[0], + file_path = f"{rom_path}/{files_to_download[0]}" + return Response( + media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"', - "Content-Type": "application/octet-stream", - "Content-Length": str(rom.file_size_bytes), + "Content-Length": str((await Path(file_path).stat()).st_size), "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_download[0]}", }, ) @@ -291,7 +301,6 @@ async def get_rom_content( media_type="application/zip", headers={ "Content-Disposition": f'attachment; filename="{quote(file_name)}.zip"', - "Content-Type": "application/zip", }, emit_body={"id": rom.id}, ) From 4f5029f78e6dcd68913db8f7d8e49a5899ce097f Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Fri, 16 Aug 2024 09:08:44 -0300 Subject: [PATCH 5/5] fix: Do not set Content-Length when X-Accel-Redirect is used --- backend/endpoints/rom.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index 2f6de260e..afe8b1c2f 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -176,7 +176,6 @@ async def head_rom_content( if not rom: raise RomNotFoundInDatabaseException(id) - rom_path = f"{LIBRARY_BASE_PATH}/{rom.full_path}" files_to_check = files or [r["filename"] for r in rom.files] if not rom.multi: @@ -184,18 +183,15 @@ async def head_rom_content( media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', - "Content-Length": str(rom.file_size_bytes), "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) if len(files_to_check) == 1: - file_path = f"{rom_path}/{files_to_check[0]}" return Response( media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(files_to_check[0])}"', - "Content-Length": str((await Path(file_path).stat()).st_size), "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_check[0]}", }, ) @@ -242,18 +238,15 @@ async def get_rom_content( media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"', - "Content-Length": str(rom.file_size_bytes), "X-Accel-Redirect": f"/library/{rom.full_path}", }, ) if len(files_to_download) == 1: - file_path = f"{rom_path}/{files_to_download[0]}" return Response( media_type="application/octet-stream", headers={ "Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"', - "Content-Length": str((await Path(file_path).stat()).st_size), "X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_download[0]}", }, )