Skip to content

Commit 4be77a4

Browse files
derekhigginsclaude
andcommitted
perf(storage): use asyncpg connection pool in PostgreSQL kvstore
Replace per-operation connect/close with asyncpg.create_pool() to avoid TCP handshake and auth overhead on every get/set/delete call. Retry once on connection error to handle stale pooled connections after a database restart. Also add pool_size and max_overflow config fields to PostgresKVStoreConfig. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Derek Higgins <derekh@redhat.com>
1 parent 778c352 commit 4be77a4

6 files changed

Lines changed: 215 additions & 68 deletions

File tree

.github/workflows/integration-sql-store-tests.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ on:
1313
- 'release-[0-9]+.[0-9]+.x'
1414
paths:
1515
- 'src/ogx/providers/utils/sqlstore/**'
16+
- 'src/ogx/core/storage/kvstore/postgres/**'
1617
- 'tests/integration/sqlstore/**'
18+
- 'tests/integration/providers/utils/kvstore/**'
1719
- 'uv.lock'
1820
- 'pyproject.toml'
1921
- 'requirements.txt'
@@ -70,6 +72,17 @@ jobs:
7072
run: |
7173
uv run pytest -sv tests/integration/providers/utils/sqlstore/
7274
75+
- name: Run KVStore Connection Resilience Test
76+
env:
77+
ENABLE_POSTGRES_TESTS: "true"
78+
POSTGRES_HOST: localhost
79+
POSTGRES_PORT: 5432
80+
POSTGRES_DB: ogx
81+
POSTGRES_USER: ogx
82+
POSTGRES_PASSWORD: ogx
83+
run: |
84+
uv run pytest -sv tests/integration/providers/utils/kvstore/
85+
7386
- name: Upload test logs
7487
if: ${{ always() }}
7588
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1

src/ogx/core/storage/datatypes.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ class PostgresKVStoreConfig(CommonConfig):
9292
ssl_mode: str | None = None
9393
ca_cert_path: str | None = None
9494
table_name: str = "ogx_kvstore"
95+
pool_size: int = Field(default=5, ge=1, description="Number of persistent connections in the pool")
96+
max_overflow: int = Field(default=10, ge=0, description="Max additional connections beyond pool_size")
9597

9698
@classmethod
9799
def sample_run_config(cls, table_name: str = "ogx_kvstore", **kwargs: object) -> dict[str, str]:

src/ogx/core/storage/kvstore/postgres/postgres.py

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
# This source code is licensed under the terms described in the LICENSE file in
55
# the root directory of this source tree.
66

7+
from collections.abc import Callable, Coroutine
78
from datetime import datetime
9+
from typing import TypeVar
810

911
import asyncpg # type: ignore[import-untyped]
1012

@@ -15,12 +17,15 @@
1517

1618
log = get_logger(name=__name__, category="providers::utils")
1719

20+
T = TypeVar("T")
21+
1822

1923
class PostgresKVStoreImpl(KVStore):
2024
"""PostgreSQL-backed key-value store implementation."""
2125

2226
def __init__(self, config: PostgresKVStoreConfig):
2327
self.config = config
28+
self._pool: asyncpg.Pool | None = None
2429
self._table_created = False
2530

2631
async def initialize(self) -> None:
@@ -35,22 +40,25 @@ def _build_ssl(self) -> object:
3540
return self.config.ssl_mode
3641
return None
3742

38-
async def _connect(self) -> asyncpg.Connection:
39-
try:
40-
conn = await asyncpg.connect(
41-
host=self.config.host,
42-
port=int(self.config.port),
43-
database=self.config.db,
44-
user=self.config.user,
45-
password=self.config.password,
46-
ssl=self._build_ssl(),
47-
)
48-
except Exception as e:
49-
log.exception("Could not connect to PostgreSQL database server")
50-
raise RuntimeError("Could not connect to PostgreSQL database server") from e
43+
async def _acquire(self) -> asyncpg.Pool:
44+
if self._pool is None:
45+
try:
46+
self._pool = await asyncpg.create_pool(
47+
host=self.config.host,
48+
port=int(self.config.port),
49+
database=self.config.db,
50+
user=self.config.user,
51+
password=self.config.password,
52+
ssl=self._build_ssl(),
53+
min_size=self.config.pool_size,
54+
max_size=self.config.pool_size + self.config.max_overflow,
55+
)
56+
except Exception as e:
57+
log.exception("Could not connect to PostgreSQL database server")
58+
raise RuntimeError("Could not connect to PostgreSQL database server") from e
5159

5260
if not self._table_created:
53-
try:
61+
async with self._pool.acquire() as conn:
5462
await conn.execute(
5563
f"""
5664
CREATE TABLE IF NOT EXISTS {self.config.table_name} (
@@ -61,10 +69,20 @@ async def _connect(self) -> asyncpg.Connection:
6169
"""
6270
)
6371
self._table_created = True
64-
except Exception:
65-
await conn.close()
66-
raise
67-
return conn
72+
73+
return self._pool
74+
75+
async def _execute_with_retry(self, fn: Callable[[asyncpg.Connection], Coroutine[None, None, T]]) -> T:
76+
"""Execute fn with a pooled connection, retrying once on connection error."""
77+
pool = await self._acquire()
78+
try:
79+
async with pool.acquire() as conn:
80+
return await fn(conn)
81+
except (asyncpg.exceptions.ConnectionDoesNotExistError, OSError):
82+
log.warning("PostgreSQL connection lost, expiring pool connections")
83+
await pool.expire_connections()
84+
async with pool.acquire() as conn:
85+
return await fn(conn)
6886

6987
def _namespaced_key(self, key: str) -> str:
7088
if not self.config.namespace:
@@ -78,8 +96,8 @@ def _strip_namespace(self, key: str) -> str:
7896

7997
async def set(self, key: str, value: str, expiration: datetime | None = None) -> None:
8098
key = self._namespaced_key(key)
81-
conn = await self._connect()
82-
try:
99+
100+
async def _do(conn: asyncpg.Connection) -> None:
83101
await conn.execute(
84102
f"""
85103
INSERT INTO {self.config.table_name} (key, value, expiration)
@@ -91,13 +109,13 @@ async def set(self, key: str, value: str, expiration: datetime | None = None) ->
91109
value,
92110
expiration,
93111
)
94-
finally:
95-
await conn.close()
112+
113+
await self._execute_with_retry(_do)
96114

97115
async def get(self, key: str) -> str | None:
98116
key = self._namespaced_key(key)
99-
conn = await self._connect()
100-
try:
117+
118+
async def _do(conn: asyncpg.Connection) -> str | None:
101119
row = await conn.fetchrow(
102120
f"""
103121
SELECT value FROM {self.config.table_name}
@@ -107,26 +125,25 @@ async def get(self, key: str) -> str | None:
107125
key,
108126
)
109127
return row["value"] if row else None
110-
finally:
111-
await conn.close()
128+
129+
return await self._execute_with_retry(_do)
112130

113131
async def delete(self, key: str) -> None:
114132
key = self._namespaced_key(key)
115-
conn = await self._connect()
116-
try:
133+
134+
async def _do(conn: asyncpg.Connection) -> None:
117135
await conn.execute(
118136
f"DELETE FROM {self.config.table_name} WHERE key = $1",
119137
key,
120138
)
121-
finally:
122-
await conn.close()
139+
140+
await self._execute_with_retry(_do)
123141

124142
async def values_in_range(self, start_key: str, end_key: str) -> list[str]:
125143
start_key = self._namespaced_key(start_key)
126144
end_key = self._namespaced_key(end_key)
127145

128-
conn = await self._connect()
129-
try:
146+
async def _do(conn: asyncpg.Connection) -> list[str]:
130147
rows = await conn.fetch(
131148
f"""
132149
SELECT value FROM {self.config.table_name}
@@ -138,15 +155,14 @@ async def values_in_range(self, start_key: str, end_key: str) -> list[str]:
138155
end_key,
139156
)
140157
return [row["value"] for row in rows]
141-
finally:
142-
await conn.close()
158+
159+
return await self._execute_with_retry(_do)
143160

144161
async def keys_in_range(self, start_key: str, end_key: str) -> list[str]:
145162
start_key = self._namespaced_key(start_key)
146163
end_key = self._namespaced_key(end_key)
147164

148-
conn = await self._connect()
149-
try:
165+
async def _do(conn: asyncpg.Connection) -> list[str]:
150166
rows = await conn.fetch(
151167
f"""
152168
SELECT key FROM {self.config.table_name}
@@ -158,8 +174,10 @@ async def keys_in_range(self, start_key: str, end_key: str) -> list[str]:
158174
end_key,
159175
)
160176
return [self._strip_namespace(row["key"]) for row in rows]
161-
finally:
162-
await conn.close()
177+
178+
return await self._execute_with_retry(_do)
163179

164180
async def shutdown(self) -> None:
165-
pass
181+
if self._pool:
182+
await self._pool.close()
183+
self._pool = None
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Copyright (c) The OGX Contributors.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the terms described in the LICENSE file in
5+
# the root directory of this source tree.
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Copyright (c) The OGX Contributors.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the terms described in the LICENSE file in
5+
# the root directory of this source tree.
6+
7+
import os
8+
import subprocess
9+
import time
10+
11+
import pytest
12+
13+
from ogx.core.storage.kvstore.config import PostgresKVStoreConfig
14+
from ogx.core.storage.kvstore.postgres.postgres import PostgresKVStoreImpl
15+
16+
POSTGRES_ENABLED = os.environ.get("ENABLE_POSTGRES_TESTS", "").lower() == "true"
17+
18+
pytestmark = pytest.mark.skipif(
19+
not POSTGRES_ENABLED,
20+
reason="PostgreSQL tests disabled (set ENABLE_POSTGRES_TESTS=true)",
21+
)
22+
23+
24+
def get_postgres_config():
25+
return PostgresKVStoreConfig(
26+
host=os.environ.get("POSTGRES_HOST", "localhost"),
27+
port=int(os.environ.get("POSTGRES_PORT", "5432")),
28+
db=os.environ.get("POSTGRES_DB", "ogx"),
29+
user=os.environ.get("POSTGRES_USER", "ogx"),
30+
password=os.environ.get("POSTGRES_PASSWORD", "ogx"),
31+
table_name="test_kvstore_resilience",
32+
)
33+
34+
35+
def _find_postgres_container():
36+
"""Find the running postgres container ID."""
37+
result = subprocess.run(
38+
["docker", "ps", "-q", "--filter", "ancestor=postgres:15"],
39+
capture_output=True,
40+
text=True,
41+
)
42+
container_id = result.stdout.strip()
43+
if not container_id:
44+
# Fallback: match any postgres container
45+
result = subprocess.run(
46+
["docker", "ps", "-q", "--filter", "name=postgres"],
47+
capture_output=True,
48+
text=True,
49+
)
50+
container_id = result.stdout.strip()
51+
if not container_id:
52+
pytest.skip("No postgres container found to restart")
53+
# Take first if multiple
54+
return container_id.split("\n")[0]
55+
56+
57+
def _restart_postgres(container_id: str, timeout: int = 30):
58+
"""Restart the postgres container and wait for it to accept connections."""
59+
subprocess.run(["docker", "restart", container_id], check=True)
60+
deadline = time.time() + timeout
61+
while time.time() < deadline:
62+
result = subprocess.run(
63+
[
64+
"docker",
65+
"exec",
66+
container_id,
67+
"pg_isready",
68+
"-U",
69+
"ogx",
70+
],
71+
capture_output=True,
72+
)
73+
if result.returncode == 0:
74+
return
75+
time.sleep(1)
76+
raise TimeoutError("PostgreSQL did not become ready after restart")
77+
78+
79+
async def test_kvstore_recovers_after_postgres_restart():
80+
config = get_postgres_config()
81+
store = PostgresKVStoreImpl(config)
82+
await store.initialize()
83+
84+
# Baseline: kvstore works
85+
await store.set("test_key", "before_restart")
86+
value = await store.get("test_key")
87+
assert value == "before_restart"
88+
89+
# Restart postgres
90+
container_id = _find_postgres_container()
91+
_restart_postgres(container_id)
92+
93+
# After restart, kvstore should still work.
94+
# Without reconnect logic this will raise InterfaceError.
95+
await store.set("test_key", "after_restart")
96+
value = await store.get("test_key")
97+
assert value == "after_restart"
98+
99+
await store.shutdown()

0 commit comments

Comments
 (0)