From e3f5117a86ac2481138c872d4e789dd538df1b3c Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Mon, 3 Mar 2025 12:28:35 +0100 Subject: [PATCH 1/3] Add db rollback on gevent timeout error --- server/mergin/sync/public_api_controller.py | 8 +++++ server/mergin/tests/test_middleware.py | 40 ++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 5aada5b6..45957482 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -13,6 +13,7 @@ import uuid from datetime import datetime +import gevent import psycopg2 from blinker import signal from connexion import NoContent, request @@ -951,6 +952,9 @@ def project_push(namespace, project_name): f"Failed to upload a new project version using transaction id: {upload.id}: {str(err)}" ) abort(422, "Failed to upload a new project version. Please try later.") + except gevent.timeout.Timeout: + db.session.rollback() + raise finally: upload.clear() @@ -1165,6 +1169,10 @@ def push_finish(transaction_id): f"transaction id: {transaction_id}.: {str(err)}" ) abort(422, "Failed to create new version: {}".format(str(err))) + # catch exception during pg transaction so we can rollback and prevent PendingRollbackError during upload clean up + except gevent.timeout.Timeout: + db.session.rollback() + raise finally: # remove artifacts upload.clear() diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index 09f4bf05..aa7805fc 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -3,9 +3,11 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import gevent +import psycogreen.gevent import pytest +import sqlalchemy -from ..app import create_simple_app, GeventTimeoutMiddleware +from ..app import create_simple_app, GeventTimeoutMiddleware, db from ..config import Configuration @@ -31,3 +33,39 @@ def ping(): if use_middleware else 200 ) + + +def test_catch_timeout(): + """Test proper handling of gevent timeout with db.session.rollback""" + psycogreen.gevent.patch_psycopg() + Configuration.GEVENT_WORKER = True + Configuration.GEVENT_REQUEST_TIMEOUT = 1 + application = create_simple_app() + + def unhandled(): + try: + db.session.execute("SELECT pg_sleep(1.1);") + finally: + db.session.execute("SELECT 1;") + return "" + + def timeout(): + try: + db.session.execute("SELECT pg_sleep(1.1);") + except gevent.timeout.Timeout: + db.session.rollback() + raise + finally: + db.session.execute("SELECT 1;") + return "" + + application.add_url_rule("/unhandled", "unhandled", unhandled) + application.add_url_rule("/timeout", "timeout", timeout) + app_context = application.app_context() + app_context.push() + + assert application.test_client().get("/timeout").status_code == 504 + + # in case of missing rollback sqlalchemy would raise error + with pytest.raises(sqlalchemy.exc.PendingRollbackError): + application.test_client().get("/unhandled") From 32005ef63939600294cc53522dd3df1778901d4e Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Mon, 3 Mar 2025 13:29:17 +0100 Subject: [PATCH 2/3] Fix tests --- server/mergin/tests/test_middleware.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index aa7805fc..3da39bc8 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -69,3 +69,5 @@ def timeout(): # in case of missing rollback sqlalchemy would raise error with pytest.raises(sqlalchemy.exc.PendingRollbackError): application.test_client().get("/unhandled") + + db.session.rollback() \ No newline at end of file From 08f395eb5d79f9206d54c5309a7e8e4c1f970087 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Mon, 3 Mar 2025 13:30:20 +0100 Subject: [PATCH 3/3] black --- server/mergin/tests/test_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index 3da39bc8..508334d0 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -69,5 +69,5 @@ def timeout(): # in case of missing rollback sqlalchemy would raise error with pytest.raises(sqlalchemy.exc.PendingRollbackError): application.test_client().get("/unhandled") - - db.session.rollback() \ No newline at end of file + + db.session.rollback()