Sitelet https://github.com/DeusMaximus/TLG-IPAM/commit/fefcf51b26df59c0f9ac10c5306efdbb476bc03f
Skip to content

Commit fefcf51

Browse files
DeusMaximusclaude
DeusMaximus
and
claude
committed
Fix build-review rough edges: WAL, allocation retry, batched subnet stats
- SQLite connections now set journal_mode=WAL + busy_timeout=5000 so concurrent UI/MCP writes queue instead of raising "database is locked" - allocate_next retries (up to 5x) when a concurrent writer claims the chosen IP, rolling to the next free candidate; race covered by test - list_hosts search uses ilike for IP/MAC (uppercase MAC queries now match) - Host.device_type widened String(16) -> String(40) to match DeviceType.name - bulk_create_hosts docstring now includes desktop in the device-type list - New batched subnets_out serialiser replaces per-subnet allocated/reserved queries (two queries total) in the REST list endpoint and MCP list_subnets - Brief updated: WAL pragmas and allocation retry behaviour Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 11c2285 commit fefcf51

9 files changed

Lines changed: 138 additions & 41 deletions

File tree

‎TLG-IPAM-Project-Brief.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ An IP is **free** iff all of the following hold:
229229

230230
Manual host creation (`POST /hosts`) only rejects the network/broadcast addresses — placing a host at the gateway, inside the DHCP pool, or inside a reserved range is deliberate and allowed.
231231

232-
`next-free` allocates the lowest free IP and creates the host record in a single transaction. The `UNIQUE(subnet_id, ip)` constraint is the backstop against races.
232+
`next-free` allocates the lowest free IP and creates the host record in a single transaction. The `UNIQUE(subnet_id, ip)` constraint is the backstop against races: if a concurrent writer claims the chosen IP first, allocation re-reads the free list and rolls to the next candidate (up to 5 attempts) rather than surfacing a conflict.
233233

234234
Gateway, DHCP, reserved-range, and allocated-address exclusions are represented as clipped, merged integer intervals. Utilisation counts interval lengths, and free-IP iteration skips blocked spans rather than materialising every address in a range.
235235

@@ -428,7 +428,7 @@ Follow this sequence to stay unblocked. Each step is independently runnable and
428428
429429
| Decision | Rationale |
430430
|---|---|
431-
| SQLite not Postgres | Zero ops overhead. A homelab IPAM will never have concurrent write contention. Backup = copy one file. |
431+
| SQLite not Postgres | Zero ops overhead. A homelab IPAM will never have sustained write contention. Backup = copy one file. Connections run with `journal_mode=WAL` and `busy_timeout=5000` so the occasional simultaneous UI/MCP write queues instead of raising "database is locked". |
432432
| FastAPI not Flask | Native async, automatic OpenAPI schema generation, Pydantic validation built in. |
433433
| FastMCP with curated tools, not OpenAPI auto-generation | Hand-written tools have better names, parameters, and denormalised answers — materially better Claude ergonomics. Shared `services/` layer eliminates the drift risk that auto-generation was meant to solve. FastMCP is the actively-maintained de facto standard. |
434434
| Streamable HTTP not stdio | Works with mcp-remote for Claude Desktop. Also compatible with any future MCP client without modification. |

‎api/app/database.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,13 @@
99

1010

1111
@event.listens_for(engine.sync_engine, "connect")
12-
def _enable_foreign_keys(dbapi_connection, _record):
12+
def _configure_connection(dbapi_connection, _record):
1313
cursor = dbapi_connection.cursor()
1414
cursor.execute("PRAGMA foreign_keys=ON")
15+
# WAL + busy_timeout so concurrent UI/API/MCP writes queue instead of
16+
# failing with "database is locked".
17+
cursor.execute("PRAGMA journal_mode=WAL")
18+
cursor.execute("PRAGMA busy_timeout=5000")
1519
cursor.close()
1620

1721

‎api/app/mcp_server.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ async def list_subnets(section_id: str | None = None) -> dict:
129129
utilisation (used/usable/free) for every subnet in one call."""
130130
async with SessionLocal() as session:
131131
subnets = await subnets_svc.list_subnets(session, section_id)
132-
return {"subnets": [await _subnet(session, s) for s in subnets]}
132+
return {"subnets": await subnets_svc.subnets_out(session, subnets)}
133133

134134

135135
@ipam_tool
@@ -243,7 +243,7 @@ async def bulk_create_hosts(subnet_id: str, hosts: list[dict]) -> dict:
243243
"""Create many host records in one subnet in a single call — use this instead of
244244
repeated create_host calls when importing or registering several devices at once
245245
(max 1000 per call). Each item is an object with: ip (required), hostname, domain,
246-
mac, device_type (server|vm|lxc|network|iot|laptop|phone|other), status
246+
mac, device_type (server|vm|lxc|network|iot|desktop|laptop|phone|other), status
247247
(active|reserved|dhcp|offline), owner, notes. Records succeed or fail
248248
individually: duplicates are skipped and invalid records report an error without
249249
aborting the batch. Returns created/skipped/error counts and a per-record result

‎api/app/models.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Host(Base):
8989
# domain suffix override; null inherits the subnet's default domain
9090
domain: Mapped[str | None] = mapped_column(String(253))
9191
mac: Mapped[str | None] = mapped_column(String(17))
92-
device_type: Mapped[str] = mapped_column(String(16), default="other")
92+
device_type: Mapped[str] = mapped_column(String(40), default="other")
9393
status: Mapped[str] = mapped_column(String(16), default="active")
9494
owner: Mapped[str | None] = mapped_column(String(120))
9595
notes: Mapped[str | None] = mapped_column(Text)

‎api/app/routers/subnets.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ async def list_subnets(
2424
section_id: str | None = None, session: AsyncSession = Depends(get_session)
2525
):
2626
subnets = await svc.list_subnets(session, section_id)
27-
data = [await _out(session, subnet) for subnet in subnets]
27+
data = await svc.subnets_out(session, subnets)
2828
return {"data": data, "total": len(data)}
2929

3030

‎api/app/services/hosts.py‎

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ async def list_hosts(
6060
if q:
6161
pattern = f"%{q}%"
6262
query = query.where(
63-
or_(Host.hostname.ilike(pattern), Host.ip.like(pattern), Host.mac.like(pattern))
63+
or_(Host.hostname.ilike(pattern), Host.ip.ilike(pattern), Host.mac.ilike(pattern))
6464
)
6565
return list((await session.scalars(query)).all())
6666

@@ -271,30 +271,50 @@ async def bulk_delete(session: AsyncSession, host_ids: list[str], source: str) -
271271
return {"deleted": len(hosts), "not_found": not_found}
272272

273273

274+
_ALLOCATE_ATTEMPTS = 5
275+
276+
274277
async def allocate_next(
275278
session: AsyncSession, subnet_id: str, payload: AllocateRequest, source: str
276279
) -> Host:
277-
subnet = await get_subnet(session, subnet_id)
278-
allocated = await _allocated_ints(session, subnet_id)
279-
reserved = await reserved_ranges(session, subnet_id)
280-
candidates = iter_free(subnet, allocated, reserved, limit=1)
281-
if not candidates:
282-
raise conflict(f"No free IPs available in subnet {subnet.name} ({subnet.cidr})")
283-
ip = candidates[0]
284-
addr = ip_in_subnet(ip, subnet.cidr)
285280
data = payload.model_dump()
286281
data["device_type"] = await device_types_svc.resolve(session, data["device_type"])
287-
host = Host(subnet_id=subnet_id, ip=ip, ip_int=int(addr), **data)
288-
session.add(host)
289-
await _flush_host(session, host)
290-
changes.record(
291-
session,
292-
entity_type="host",
293-
entity_id=host.id,
294-
action="create",
295-
summary=f"host {_label(host)} allocated next free IP {ip} in {subnet.name}",
296-
source=source,
297-
after=changes.snapshot(host),
282+
# A concurrent writer can claim the chosen IP between the free-list read and
283+
# the insert; on that unique-constraint failure, re-read and roll to the next
284+
# free IP instead of surfacing a conflict.
285+
for _ in range(_ALLOCATE_ATTEMPTS):
286+
subnet = await get_subnet(session, subnet_id)
287+
allocated = await _allocated_ints(session, subnet_id)
288+
reserved = await reserved_ranges(session, subnet_id)
289+
candidates = iter_free(subnet, allocated, reserved, limit=1)
290+
if not candidates:
291+
raise conflict(f"No free IPs available in subnet {subnet.name} ({subnet.cidr})")
292+
ip = candidates[0]
293+
addr = ip_in_subnet(ip, subnet.cidr)
294+
host = Host(subnet_id=subnet_id, ip=ip, ip_int=int(addr), **data)
295+
session.add(host)
296+
summary = f"host {_label(host)} allocated next free IP {ip} in {subnet.name}"
297+
try:
298+
await session.flush()
299+
except IntegrityError:
300+
await session.rollback()
301+
continue
302+
changes.record(
303+
session,
304+
entity_type="host",
305+
entity_id=host.id,
306+
action="create",
307+
summary=summary,
308+
source=source,
309+
after=changes.snapshot(host),
310+
)
311+
try:
312+
await session.commit()
313+
except IntegrityError:
314+
await session.rollback()
315+
continue
316+
return host
317+
raise conflict(
318+
f"Could not allocate a free IP in subnet {subnet_id}: "
319+
f"lost the race to a concurrent allocation {_ALLOCATE_ATTEMPTS} times"
298320
)
299-
await _commit_host(session, host)
300-
return host

‎api/app/services/subnets.py‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,13 @@ async def reserved_ranges(session: AsyncSession, subnet_id: str) -> list[Reserve
4444
return list((await session.scalars(query)).all())
4545

4646

47-
async def subnet_stats(session: AsyncSession, subnet: Subnet) -> dict:
47+
def _stats(subnet: Subnet, allocated: set[int], reserved: list[ReservedRange]) -> dict:
4848
"""used = host records; usable = addresses excl. network/broadcast;
4949
free = allocatable right now (usable minus gateway, DHCP pool, reserved
5050
ranges, and allocated)."""
5151
net = parse_cidr(subnet.cidr)
5252
first, last = usable_bounds(net)
5353
usable = last - first + 1
54-
allocated = await _allocated_ints(session, subnet.id)
55-
reserved = await reserved_ranges(session, subnet.id)
5654
excluded = exclusion_intervals(subnet, reserved)
5755
blocked = merge_intervals(
5856
[*excluded, *((ip, ip) for ip in allocated)], lower=first, upper=last
@@ -61,22 +59,67 @@ async def subnet_stats(session: AsyncSession, subnet: Subnet) -> dict:
6159
return {"used": len(allocated), "usable": usable, "free": free}
6260

6361

64-
async def subnet_out(session: AsyncSession, subnet: Subnet) -> dict:
65-
"""Serialised subnet with utilisation stats and reserved ranges — the shape
66-
shared by the REST API and MCP tools."""
67-
stats = await subnet_stats(session, subnet)
68-
reserved = [
69-
ReservedRangeOut.model_validate(r).model_dump()
70-
for r in await reserved_ranges(session, subnet.id)
71-
]
62+
async def subnet_stats(session: AsyncSession, subnet: Subnet) -> dict:
63+
allocated = await _allocated_ints(session, subnet.id)
64+
reserved = await reserved_ranges(session, subnet.id)
65+
return _stats(subnet, allocated, reserved)
66+
67+
68+
async def _grouped_allocated(session: AsyncSession, subnet_ids: list[str]) -> dict[str, set[int]]:
69+
rows = await session.execute(
70+
select(Host.subnet_id, Host.ip_int).where(Host.subnet_id.in_(subnet_ids))
71+
)
72+
grouped: dict[str, set[int]] = {}
73+
for subnet_id, ip_int in rows.all():
74+
grouped.setdefault(subnet_id, set()).add(ip_int)
75+
return grouped
76+
77+
78+
async def _grouped_reserved(
79+
session: AsyncSession, subnet_ids: list[str]
80+
) -> dict[str, list[ReservedRange]]:
81+
rows = await session.scalars(
82+
select(ReservedRange)
83+
.where(ReservedRange.subnet_id.in_(subnet_ids))
84+
.order_by(ReservedRange.start_int)
85+
)
86+
grouped: dict[str, list[ReservedRange]] = {}
87+
for block in rows:
88+
grouped.setdefault(block.subnet_id, []).append(block)
89+
return grouped
90+
91+
92+
def _subnet_dict(subnet: Subnet, stats: dict, reserved: list[ReservedRange]) -> dict:
7293
# build from columns, not the ORM object: the reserved_ranges relationship would
7394
# otherwise lazy-load (not allowed under the async session)
7495
columns = {c.name: getattr(subnet, c.name) for c in subnet.__table__.columns}
96+
ranges = [ReservedRangeOut.model_validate(r).model_dump() for r in reserved]
7597
return SubnetOut.model_validate(
76-
{**columns, **stats, "reserved_ranges": reserved}
98+
{**columns, **stats, "reserved_ranges": ranges}
7799
).model_dump(mode="json")
78100

79101

102+
async def subnets_out(session: AsyncSession, subnets: list[Subnet]) -> list[dict]:
103+
"""Serialised subnets with utilisation stats and reserved ranges — the shape
104+
shared by the REST API and MCP tools. Batches the per-subnet host and
105+
reserved-range lookups into two queries regardless of subnet count."""
106+
ids = [s.id for s in subnets]
107+
allocated = await _grouped_allocated(session, ids) if ids else {}
108+
reserved = await _grouped_reserved(session, ids) if ids else {}
109+
return [
110+
_subnet_dict(
111+
s,
112+
_stats(s, allocated.get(s.id, set()), reserved.get(s.id, [])),
113+
reserved.get(s.id, []),
114+
)
115+
for s in subnets
116+
]
117+
118+
119+
async def subnet_out(session: AsyncSession, subnet: Subnet) -> dict:
120+
return (await subnets_out(session, [subnet]))[0]
121+
122+
80123
async def free_ips(session: AsyncSession, subnet_id: str, limit: int = 100) -> list[str]:
81124
subnet = await get_subnet(session, subnet_id)
82125
allocated = await _allocated_ints(session, subnet_id)

‎api/tests/test_allocation.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,30 @@ async def test_hostname_required_for_allocation(client):
6868
subnet = await make_subnet(client, section["id"])
6969
res = await client.post(f"/api/v1/subnets/{subnet['id']}/next-free", json={})
7070
assert res.status_code == 422
71+
72+
73+
async def test_allocation_retries_past_racing_writer(client, monkeypatch):
74+
"""If another writer claims the chosen IP between the free-list read and the
75+
insert, allocation rolls to the next free IP instead of raising a conflict."""
76+
from app.services import hosts as hosts_svc
77+
78+
section = await make_section(client)
79+
subnet = await make_subnet(client, section["id"], "10.0.0.0/29")
80+
await make_host(client, subnet["id"], "10.0.0.1")
81+
82+
real = hosts_svc._allocated_ints
83+
calls = {"count": 0}
84+
85+
async def stale_then_real(session, subnet_id):
86+
calls["count"] += 1
87+
if calls["count"] == 1:
88+
return set() # stale read: hides 10.0.0.1, simulating a lost race
89+
return await real(session, subnet_id)
90+
91+
monkeypatch.setattr(hosts_svc, "_allocated_ints", stale_then_real)
92+
res = await client.post(
93+
f"/api/v1/subnets/{subnet['id']}/next-free", json={"hostname": "raced"}
94+
)
95+
assert res.status_code == 201, res.text
96+
assert res.json()["data"]["ip"] == "10.0.0.2"
97+
assert calls["count"] >= 2

‎api/tests/test_hosts.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ async def test_search_and_filters(client):
5959
by_mac = (await client.get("/api/v1/hosts?q=dd:ee")).json()
6060
assert by_mac["total"] == 1
6161

62+
by_mac_upper = (await client.get("/api/v1/hosts?q=DD:EE")).json()
63+
assert by_mac_upper["total"] == 1
64+
6265
offline = (await client.get("/api/v1/hosts?status=offline")).json()
6366
assert offline["total"] == 1 and offline["data"][0]["status"] == "offline"
6467

0 commit comments

Comments
 (0)