Improve DB connections handling

AGENTS.md

@@ -1,7 +1,7 @@
 # Repository Guidelines
 
 Before touching a subsystem, read the relevant notes in `contributing/`: `ARCHITECTURE.md`,
-`PIPELINES.md`, `LOCKING.md`, `MIGRATIONS.md`, `RUNS-AND-JOBS.md`, `AUTOSCALING.md`,
+`PIPELINES.md`, `LOCKING.md`, `DATABASE.md`, `MIGRATIONS.md`, `RUNS-AND-JOBS.md`, `AUTOSCALING.md`,
 `BACKENDS.md`, `GPUHUNT.md`, `PROXY.md`, `RUNNER-AND-SHIM.md`, `FRONTEND.md`, `DOCS.md`,
 `DEVELOPMENT.md`, `RELEASE.md`.
 
@@ -29,7 +29,7 @@ Before touching a subsystem, read the relevant notes in `contributing/`: `ARCHIT
 - Prefer pydantic-style models in `core/models`.
 - Document attributes when the note adds behavior, compatibility, or semantic context that is not obvious from the name and type. Use attribute docstrings without leading newline.
 - Tests use `test_*.py` modules and `test_*` functions; fixtures live near usage.
-- Never make network calls inside a DB session or transaction. Fetch what you need before opening the session, or commit and close it before the call.
+- Never make network calls inside a DB session or transaction. Fetch what you need before opening the session, or commit and close it before the call. See `contributing/DATABASE.md`.
 - Don't use function-level (inner) imports to break circular imports. Inject the dependency or move the shared code to a lower-level module instead.
 - Never edit a migration that has already been applied or released; add a new migration instead.
 - Derive paths under `SERVER_DIR_PATH` on access (a `get_*` function), not as module-level constants, so that patching `settings.SERVER_DIR_PATH` redirects all of them. Tests rely on this to keep server state out of the real `~/.dstack`.

contributing/DATABASE.md

@@ -0,0 +1,86 @@
+# Database
+
+This document describes the `dstack` server rules of working with the database. Separately covered:
+
+* DB migrations in `MIGRATIONS.md`
+* Resource locking in `LOCKING.md`
+* Pipeline-based background processing in `PIPELINES.md`.
+
+## Connections
+
+DB connections are a scarce resource. For example, a minimal RDS Postgres instance comes with ~80 connections. Increasing the number of connections requires
+scaling the DB instance and is expensive. This section describes how the `dstack` server should use DB connections to avoid running out of them.
+
+### Connection budget
+
+The server creates one SQLAlchemy async engine per process with an `AsyncAdaptedQueuePool` of `DSTACK_DB_POOL_SIZE` (default 20) persistent connections plus `DSTACK_DB_MAX_OVERFLOW` (default 20) overflow connections, i.e. up to 40 connections per server process. When all of them are checked out, the next checkout waits `pool_timeout` (30 seconds) and then fails with:
+
+```
+sqlalchemy.exc.TimeoutError: QueuePool limit of size 20 overflow 20 reached, connection timed out, timeout 30.00
+```
+
+Far more coroutines compete for the slots than there are slots. This works because each of them holds a connection for a short time.
+
+A session does not hold a connection until it executes its first statement. From that moment until the session commits, rolls back, or closes, the connection is checked out and unavailable to anyone else. The invariant that follows:
+
+**Hold a connection only while doing DB work. Do not keep a session open across when doing heavy work.**
+
+On SQLite the same rule applies for a different reason: a long write transaction blocks every other writer for up to `busy_timeout` (30 seconds).
+
+### Awaits that must not run inside a session
+
+Anything whose completion depends on something other than the database:
+
+* SSH: opening tunnels, `aexec()`, `acheck()`, any subprocess.
+* HTTP to runners, shims, gateways, or external services.
+* Cloud SDK calls, including those wrapped in `run_async()`. The thread pool executor is shared, so a saturated executor makes even a fast call wait.
+* Retry loops with sleeps, and waiting on asyncio locks or events that other tasks control.
+* Streaming a response to a client.
+
+Fetch what you need before opening the session, or commit and close the session before the call and open a new one to apply the results:
+
+```python
+async with get_session_ctx() as session:
+    # Load and lock what processing needs.
+    context = await _load_context(session, item)
+
+# No connection is held here.
+result = await _do_network_work(context)
+
+async with get_session_ctx() as session:
+    # Apply results with a guarded update.
+    await _apply_result(session, item, result)
+```
+
+### Session lifetime by context
+
+**Pipelines**
+
+Pipeline workers follow load, process, apply with a short session for the load and apply phases and no session during processing. See `PIPELINES.md`.
+
+**API handlers**
+
+The request session is created before the handler runs. The authentication dependency executes a SELECT on it, so a connection is checked out before the handler starts and stays checked out until the handler returns and the session commits.
+
+This is acceptable because API handlers are assumed to be quick: read or write a few rows and return. A handler that needs to do slow work must not do it while holding the request session's connection. Either:
+
+* Commit the session before the slow work. Committing returns the connection to the pool. The next statement on the session checks out a new connection and starts a new transaction, so any locks taken earlier are released, and the handler must not rely on them across the gap.
+* Make the API async and move the slow work out of the handler.
+
+The rule is not applied currently applied to API handlers strictly: a handler may make one or a few network calls while holding the connection.
+
+**Startup**
+
+Migrations and server initialization may hold a connection and a session-level advisory lock for long: they run once per process, before anything else competes for connections. Recurring cross-replica coordination should use the pipeline lock columns instead.
+
+### Lock waits
+
+A coroutine blocked on a DB-level lock holds its connection while it waits. This applies to `SELECT ... FOR UPDATE` and to `pg_advisory_lock` alike: the waiter occupies a pool slot on its replica until the holder releases the lock. A lock held for a long time thus costs one slot on the holder plus one per waiter, and a hung holder pins all of them.
+
+Keep locked regions short and bounded. See `LOCKING.md` for how to take advisory locks correctly.
+
+### Settings
+
+`DSTACK_DB_COMMAND_TIMEOUT` (default 300 seconds) bounds every operation on a Postgres connection, so a coroutine waiting on a connection whose socket died silently gets an error and releases the slot instead of holding it forever. It applies to migrations too, since they run on the same engine, so it must leave room for index builds and backfills.
+
+`DSTACK_DB_POOL_SIZE` and `DSTACK_DB_MAX_OVERFLOW` size the pool per process. Raising them lets more coroutines hold connections at once, which hides a long hold rather than fixing it, and each replica needs `pool_size + max_overflow` connections from the Postgres `max_connections` budget.

contributing/LOCKING.md

@@ -29,7 +29,7 @@ Locksets are an optimization. One can think of them as per-resource-id locks tha
 Postgres resource locking is implemented via standard SELECT FOR UPDATE.
 SQLAlchemy provides `.with_for_update()` that has no effect if SELECT FOR UPDATE is not supported as in SQLite.
 
-There are few places that rely on advisory locks as when generating unique resource names.
+There are few places that rely on advisory locks as when generating unique resource names or serializing server initialization across replicas. See **Advisory locks** below.
 
 ## Working with locks
 
@@ -112,6 +112,39 @@ Note that:
 * This pattern works assuming that Postgres is using default isolation level Read Committed. By the time a transaction acquires the advisory lock, all other transactions that can take the name have committed, so their changes can be seen and a unique name is taken.
 * SQLite needs a commit before selecting taken names due to Snapshot Isolation as noted above.
 
+**Advisory locks**
+
+Postgres has two kinds of advisory locks:
+
+* `pg_advisory_xact_lock` is released when the transaction ends. The unique names pattern above uses it.
+* `pg_advisory_lock` is bound to the connection. It survives commit and rollback and is released only by `pg_advisory_unlock` on the same connection or when the connection closes. `advisory_lock_ctx()` in `services/locking.py` wraps this kind for work that must span several transactions, such as migrations or server initialization.
+
+Within a single transaction, prefer `pg_advisory_xact_lock` for automatic release on commit or rollback. When using `advisory_lock_ctx()`, the requirement is to acquire and release the lock on the same physical connection:
+
+* An `AsyncSession` is a valid bind when the locked block stays within one transaction. Do not commit, roll back, or close the session before releasing the lock.
+* If the block spans multiple transactions, hold an `AsyncConnection` from `engine.connect()` and bind the lock and any sessions to it. Otherwise, a session's next transaction may use a different pooled connection, and `pg_advisory_unlock` goes to a connection that never held the lock. Postgres only returns `false` with a warning in this case, so the failure is silent: the lock stays on an idle pooled connection until the process exits, and every replica blocks forever on its next acquire. See https://github.com/dstackai/dstack/issues/3881 for an example.
+* Run protected DB work on the connection holding the lock. When binding a new session to an explicit connection, end the lock statement's transaction first so that the session controls its own transactions. If the connection is lost, abort the protected operation instead of reconnecting and continuing without the lock.
+* Keep the locked block short and bounded. Every waiter is blocked inside `pg_advisory_lock` holding a DB connection of its own, so a long or hung holder pins one connection per waiter. See `DATABASE.md`.
+
+For a block that spans multiple transactions:
+
+```python
+async with get_db().engine.connect() as connection:
+    async with advisory_lock_ctx(
+        bind=connection,
+        dialect_name=get_db().dialect_name,
+        resource="server_init",
+    ):
+        # End the lock statement's transaction. The session-level lock survives the commit.
+        await connection.commit()
+        async with get_db().get_session(bind=connection) as session:
+            # Session commits keep using the connection that holds the lock.
+            ...
+            await session.commit()
+```
+
+A released connection goes back to the pool, so a lock that failed to release stays there too. `_release_advisory_lock()` tolerates failures because the common one is an invalidated connection, in which case Postgres has already dropped the lock. A release that fails on a live connection strands the lock.
+
 **Use `AsyncExitStack`**
 
 In-memory locking typically requires taking lock for long (until commit).

mkdocs/docs/reference/env.md

@@ -141,6 +141,7 @@ For more details on the options below, refer to the [server deployment](../guide
 - `DSTACK_SERVER_GCS_BUCKET`{ #DSTACK_SERVER_GCS_BUCKET } - The bucket that repo diffs will be uploaded to if set. If unset, diffs are uploaded to the database.
 - `DSTACK_DB_POOL_SIZE`{ #DSTACK_DB_POOL_SIZE } - The client DB connections pool size. Defaults to `20`,
 - `DSTACK_DB_MAX_OVERFLOW`{ #DSTACK_DB_MAX_OVERFLOW } - The client DB connections pool allowed overflow. Defaults to `20`.
+- `DSTACK_DB_COMMAND_TIMEOUT`{ #DSTACK_DB_COMMAND_TIMEOUT } - The timeout for a single DB operation in seconds, Postgres only. Set to `0` to disable. Defaults to `300`.
 - `DSTACK_SERVER_BACKGROUND_PROCESSING_DISABLED`{ #DSTACK_SERVER_BACKGROUND_PROCESSING_DISABLED } - Disables background processing if set to any value. Useful to run only web frontend and API server.
 - `DSTACK_SERVER_MAX_PROBES_PER_JOB`{ #DSTACK_SERVER_MAX_PROBES_PER_JOB } - Maximum number of probes allowed in a run configuration. Validated at apply time.
 - `DSTACK_SERVER_MAX_PROBE_TIMEOUT`{ #DSTACK_SERVER_MAX_PROBE_TIMEOUT } - Maximum allowed timeout for a probe. Validated at apply time.

src/dstack/_internal/server/app.py

@@ -23,7 +23,7 @@
 from dstack._internal.server.background.pipeline_tasks import start_pipeline_tasks
 from dstack._internal.server.background.scheduled_tasks import start_scheduled_tasks
 from dstack._internal.server.background.scheduled_tasks.probes import PROBES_SCHEDULER
-from dstack._internal.server.db import get_db, get_session_ctx, migrate
+from dstack._internal.server.db import get_db, migrate
 from dstack._internal.server.routers import (
     auth,
     backends,
@@ -130,33 +130,43 @@ async def lifespan(app: FastAPI):
         server_config_loaded = server_config_manager.load_config()
         # Encryption has to be configured before working with users and projects
         await server_config_manager.apply_encryption()
-    async with get_session_ctx() as session:
+    async with get_db().engine.connect() as connection:
+        # Running server init using a dedicated connection because there are multiple
+        # transactions/commits happening under the advisory lock
+        # and we need to guarantee the same connection releases the lock.
         async with advisory_lock_ctx(
-            bind=session,
+            bind=connection,
             dialect_name=get_db().dialect_name,
             resource="server_init",
         ):
-            admin, _ = await get_or_create_admin_user(session=session)
-            await get_or_create_default_project(
-                session=session,
-                user=admin,
-            )
-            if server_config_manager is not None:
-                server_config_file_path = get_server_config_file_path()
-                server_config_dir = _get_server_config_dir()
-                if not server_config_loaded:
-                    logger.info("Initializing the default configuration...", {"show_path": False})
-                    await server_config_manager.init_config(session=session)
-                    logger.info(
-                        f"Initialized the default configuration at [link=file://{server_config_file_path}]{server_config_dir}[/link]",
-                        {"show_path": False},
-                    )
-                else:
-                    logger.info(
-                        f"Applying [link=file://{server_config_file_path}]{server_config_dir}[/link]...",
-                        {"show_path": False},
-                    )
-                    await server_config_manager.apply_config(session=session, owner=admin)
+            # End the lock statement's transaction so that the session controls its own transactions.
+            # The session-level lock survives the commit.
+            await connection.commit()
+            async with get_db().get_session(bind=connection) as session:
+                admin, _ = await get_or_create_admin_user(session=session)
+                await get_or_create_default_project(
+                    session=session,
+                    user=admin,
+                )
+                if server_config_manager is not None:
+                    server_config_file_path = get_server_config_file_path()
+                    server_config_dir = _get_server_config_dir()
+                    if not server_config_loaded:
+                        logger.info(
+                            "Initializing the default configuration...", {"show_path": False}
+                        )
+                        await server_config_manager.init_config(session=session)
+                        logger.info(
+                            f"Initialized the default configuration at [link=file://{server_config_file_path}]{server_config_dir}[/link]",
+                            {"show_path": False},
+                        )
+                    else:
+                        logger.info(
+                            f"Applying [link=file://{server_config_file_path}]{server_config_dir}[/link]...",
+                            {"show_path": False},
+                        )
+                        await server_config_manager.apply_config(session=session, owner=admin)
+                await session.commit()
 
     update_default_project(
         project_name=DEFAULT_PROJECT_NAME,

src/dstack/_internal/server/background/pipeline_tasks/runs/__init__.py

@@ -18,7 +18,8 @@
 import dstack._internal.server.background.pipeline_tasks.runs.active as active
 import dstack._internal.server.background.pipeline_tasks.runs.pending as pending
 import dstack._internal.server.background.pipeline_tasks.runs.terminating as terminating
-from dstack._internal.core.models.runs import JobStatus, RunStatus
+from dstack._internal.core.models.runs import JobStatus, RunSpec, RunStatus
+from dstack._internal.proxy.gateway.schemas.stats import PerWindowStats
 from dstack._internal.server.background.pipeline_tasks.base import (
     Fetcher,
     Heartbeater,
@@ -300,6 +301,7 @@ async def _process_pending_item(item: RunPipelineItem) -> None:
         if context is None:
             return
 
+    context.gateway_stats = await _load_gateway_stats(context.run_model, context.run_spec)
     result = await pending.process_pending_run(context)
     if result is None:
         await _apply_noop_result(
@@ -330,21 +332,11 @@ async def _load_pending_context(
         return None
     secrets = await get_project_secrets_mapping(session=session, project=run_model.project)
     run_spec = get_run_spec(run_model)
-
-    gateway_stats = None
-    if run_spec.configuration.type == "service" and run_model.gateway is not None:
-        gateway_stats = await get_combined_gateway_stats(
-            get_gateway_replica_models(run_model.gateway),
-            run_model.project.name,
-            run_model.run_name,
-        )
-
     return pending.PendingContext(
         run_model=run_model,
         run_spec=run_spec,
         secrets=secrets,
         locked_job_ids=locked_job_ids,
-        gateway_stats=gateway_stats,
     )
 
 
@@ -506,6 +498,7 @@ async def _process_active_item(item: RunPipelineItem) -> None:
             return
         context = load_result
 
+    context.gateway_stats = await _load_gateway_stats(context.run_model, context.run_spec)
     result = await active.process_active_run(context)
     await _apply_active_result(item=item, context=context, result=result)
 
@@ -532,21 +525,21 @@ async def _load_active_context(
         return None
     secrets = await get_project_secrets_mapping(session=session, project=run_model.project)
     run_spec = get_run_spec(run_model)
-
-    gateway_stats = None
-    if run_spec.configuration.type == "service" and run_model.gateway is not None:
-        gateway_stats = await get_combined_gateway_stats(
-            get_gateway_replica_models(run_model.gateway),
-            run_model.project.name,
-            run_model.run_name,
-        )
-
     return active.ActiveContext(
         run_model=run_model,
         run_spec=run_spec,
         secrets=secrets,
         locked_job_ids=locked_job_ids,
-        gateway_stats=gateway_stats,
+    )
+
+
+async def _load_gateway_stats(run_model: RunModel, run_spec: RunSpec) -> Optional[PerWindowStats]:
+    if run_spec.configuration.type != "service" or run_model.gateway is None:
+        return None
+    return await get_combined_gateway_stats(
+        get_gateway_replica_models(run_model.gateway),
+        run_model.project.name,
+        run_model.run_name,
     )