Skip to content

Commit 314912d

Browse files
fregataaclaude
andcommitted
refactor(BA-7353): move the kernel payload into common/interchange and carry only what the manager reads
Review follow-up. `common/data` is where a component keeps its own value objects, so a payload that crosses a component boundary had no documented home — `dto/` is organized by target component, and a payload has two sides. - Add `common/interchange/`, with `AGENTS.md` stating the membership test: one component produces it, another consumes it, so it has to survive serialization. Record the split between `interchange` / `dto` / `schema` / `data` in `common/KNOWLEDGE.md`, and list the package (along with the previously missing `schema/`) in `common/AGENTS.md`. - Carry only what `update_kernel_status_running()` reads, which is its one consumer: `container_id`, `kernel_host`, the two repl ports, `service_ports`, `attached_devices` and `allocations`. Dropped `id` (the event already names the kernel), `agent_addr` (the event names its source), the legacy stdio ports, and the parts of the resource spec — slots, mounts, unified devices, scratch size — that nothing reads. - Return `list[ResourceSlotEntry]` rather than the legacy `ResourceSlot`; a caller that needs the dict form goes through `ResourceSlotEntry.inputs_to_resource_slot()`. - Name the levels of the allocation mapping (`DeviceAllocation`, `PerDeviceAllocation`) instead of nesting three dicts inline, and apply the `DeviceAllocation` alias that `agent/resources.py` already defined but did not use. With the spec no longer travelling whole, `KernelResourceSpecData` / `MountData` / `to_data()` would only serve the agent's own log, so they are gone and `to_json_serializable_dict()` is left as it was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a575d7d commit 314912d

12 files changed

Lines changed: 311 additions & 340 deletions

File tree

src/ai/backend/agent/agent.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2713,7 +2713,7 @@ async def create_kernel(
27132713
"create_kernel(kernel:{}, session:{}) resource spec prepared: {}",
27142714
kernel_id,
27152715
session_id,
2716-
resource_spec.to_data().model_dump_json(),
2716+
resource_spec.to_json(),
27172717
)
27182718

27192719
# Mount backend-specific intrinsic mounts (e.g., scratch directories)

src/ai/backend/agent/resources.py

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@
4040
InvalidResourceConfigError,
4141
ResourceOverAllocatedError,
4242
)
43-
from ai.backend.common.data.kernel.types import KernelResourceSpecData, MountData
4443
from ai.backend.common.etcd import AsyncEtcd
45-
from ai.backend.common.identifier.resource_slot import ResourceSlotName
4644
from ai.backend.common.json import dump_json_str, load_json
4745
from ai.backend.common.plugin import AbstractPlugin, BasePluginContext
4846
from ai.backend.common.types import (
@@ -57,7 +55,6 @@
5755
MountPermission,
5856
MountTypes,
5957
ResourceSlot,
60-
ResourceSlotEntry,
6158
SlotName,
6259
SlotTypes,
6360
aobject,
@@ -126,7 +123,7 @@ class KernelResourceSpec:
126123
slots: ResourceSlot
127124
"""Stores the original user-requested resource slots."""
128125

129-
allocations: MutableMapping[DeviceName, Mapping[SlotName, Mapping[DeviceId, Decimal]]]
126+
allocations: MutableMapping[DeviceName, DeviceAllocation]
130127
"""
131128
Represents the resource allocations for each slot (device) type and devices.
132129
"""
@@ -265,34 +262,35 @@ async def aread_from_file(cls, file: AsyncTextIOWrapper) -> Self:
265262
text = "\n".join(await file.readlines())
266263
return cls.read_from_string(text)
267264

268-
def to_data(self) -> KernelResourceSpecData:
269-
"""
270-
Render this spec as the value events and logs carry it as.
271-
"""
272-
return KernelResourceSpecData(
273-
slots=ResourceSlotEntry.from_resource_slot(self.slots),
274-
allocations={
275-
dev_name: {
276-
ResourceSlotName(str(slot_name)): dict(per_device_alloc)
277-
for slot_name, per_device_alloc in dev_alloc.items()
278-
}
279-
for dev_name, dev_alloc in self.allocations.items()
280-
},
281-
scratch_disk_size=self.scratch_disk_size,
282-
mounts=[
283-
MountData(
284-
type=mount.type,
285-
source=mount.source,
286-
target=mount.target,
287-
permission=mount.permission,
288-
)
289-
for mount in self.mounts
290-
],
291-
unified_devices=[
292-
(device_name, ResourceSlotName(str(slot_name)))
293-
for device_name, slot_name in self.unified_devices
294-
],
295-
)
265+
def to_json_serializable_dict(self) -> Mapping[str, Any]:
266+
o = attrs.asdict(self)
267+
for slot_name, alloc in o["slots"].items():
268+
if known_slot_types.get(slot_name, "count") == "bytes":
269+
o["slots"] = f"{BinarySize(alloc):s}"
270+
else:
271+
o["slots"] = str(alloc)
272+
serialized_allocations = {}
273+
for dev_name, dev_alloc in o["allocations"].items():
274+
serialized_dev_alloc = {}
275+
for slot_name, per_device_alloc in dev_alloc.items():
276+
serialized_per_device_alloc = {}
277+
for dev_id, alloc in per_device_alloc.items():
278+
if known_slot_types.get(slot_name, "count") == "bytes":
279+
serialized_alloc = f"{BinarySize(alloc):s}"
280+
else:
281+
serialized_alloc = str(alloc)
282+
serialized_per_device_alloc[str(dev_id)] = serialized_alloc
283+
serialized_dev_alloc[str(slot_name)] = serialized_per_device_alloc
284+
serialized_allocations[str(dev_name)] = serialized_dev_alloc
285+
o["allocations"] = serialized_allocations
286+
o["mounts"] = list(map(str, self.mounts))
287+
o["unified_devices"] = [
288+
(str(device_name), str(slot_name)) for device_name, slot_name in self.unified_devices
289+
]
290+
return o
291+
292+
def to_json(self) -> str:
293+
return dump_json_str(self.to_json_serializable_dict())
296294

297295
@classmethod
298296
def __get_pydantic_core_schema__(

src/ai/backend/common/AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
| `common/events/` | Event type definitions and dispatcher — see existing `AbstractEvent` subclasses |
2727
| `common/bgtask/` | Background task framework — extend `BaseBackgroundTaskHandler` |
2828
| `common/dto/` | Inter-component DTOs — see `common/dto/AGENTS.md` |
29+
| `common/interchange/` | Payloads one component sends another (event/RPC bodies) — see `common/interchange/AGENTS.md` |
30+
| `common/schema/` | Pydantic types the manager persists as a JSON DB column — see `common/schema/AGENTS.md` |
2931
| `common/exception.py` | Root `BackendAIError` and `ErrorCode` — all component exceptions inherit from here |
3032
| `common/types.py` | Common base types used across layers |
3133

src/ai/backend/common/KNOWLEDGE.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,20 @@ types belong here is the core of this document.
2727
- The second criterion: types that, if placed in a higher layer, **would force an upward import** (`common/schema/` states this explicitly and is used only by the manager).
2828
- Single-consumer subpackages (`stage/` — agent-only, `resilience/` — manager-only) are not defects.
2929
- "It is convenient to put it somewhere neutral" is not a membership reason.
30+
31+
## Shared types are split by what crosses, not by who owns them
32+
33+
A type shared by two components still has to land in one of four places, and the
34+
question that separates them is what boundary it crosses:
35+
36+
| Package | Crosses |
37+
|---------|---------|
38+
| `interchange/` | A component boundary as a serialized body (event/RPC payload) |
39+
| `dto/` | An API boundary, per target component |
40+
| `schema/` | The persistence boundary — the manager stores it as a JSON column |
41+
| `data/` | Nothing; it is passed within a component |
42+
43+
`interchange/` exists because the first row had no home: `dto/` is organized by target
44+
component, and a payload has two sides, so neither side owns it. Its membership test is
45+
the consumer, not the producer — a field the receiving component never reads belongs in
46+
the producing component, not on the wire.

src/ai/backend/common/data/kernel/types.py

Lines changed: 0 additions & 123 deletions
This file was deleted.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# `common/interchange` guardrails
2+
3+
The bodies components send each other. A type belongs here when one component produces it,
4+
another consumes it, and it therefore has to survive serialization — event payloads and RPC
5+
payloads.
6+
7+
## Rules
8+
9+
- **Contents**: pydantic models only. Every field must be JSON-representable and round-trip
10+
unchanged — no `Any`, and no type that only survives pickling (`ResourceSlot`, `SlotName`,
11+
`BinarySize`, `Path` used as a bare value).
12+
- **Carry what the consumer reads.** A payload mirrors the contract between the two components,
13+
not the producer's internal struct. A field no consumer reads does not belong on the wire; keep
14+
it in the producing component.
15+
- **Organized by domain**, one module per domain (`interchange/kernel.py`), not by component —
16+
a payload has two sides, so neither side owns the directory.
17+
- **Dependency direction (leaf)**: depend only on lower `common` modules (`common.types`,
18+
`common.identifier`). MUST NOT import from `manager` / `agent` / `storage`, or from
19+
`common.dto`.
20+
- **No business logic.** A conversion that only reshapes the payload's own fields is fine
21+
(`to_resource_slot_entries()`); anything that needs a repository, a config, or another entity is
22+
not.
23+
24+
## How this differs from its neighbours
25+
26+
| Package | Holds |
27+
|---------|-------|
28+
| `interchange/` | Payloads crossing a component boundary — serialized by definition |
29+
| `dto/` | API request/response contracts, per target component (`dto/manager/v2/...`) |
30+
| `schema/` | Pydantic types the manager persists as a JSON DB column |
31+
| `data/` | Value objects passed within a component |
32+
33+
When a payload type also has to be persisted as a column, it belongs in `schema/`, and
34+
`interchange/` may reference it.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AGENTS.md
File renamed without changes.
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
from __future__ import annotations
2+
3+
from decimal import Decimal
4+
from typing import Annotated
5+
6+
from pydantic import Field
7+
8+
from ai.backend.common.identifier.resource_slot import ResourceSlotName
9+
from ai.backend.common.types import (
10+
BackendAISchema,
11+
ContainerId,
12+
DeviceId,
13+
DeviceName,
14+
ResourceSlotEntry,
15+
ServicePortProtocols,
16+
)
17+
18+
__all__ = (
19+
"AttachedDeviceData",
20+
"DeviceAllocation",
21+
"DeviceCapacityData",
22+
"KernelCreationInfo",
23+
"PerDeviceAllocation",
24+
"ServicePortData",
25+
)
26+
27+
28+
type AllocationAmount = Annotated[
29+
Decimal,
30+
# An unbounded allocation is expressed as `Decimal("Infinity")`, which the default
31+
# `Decimal` constraint rejects. It survives the wire as the string "Infinity".
32+
Field(allow_inf_nan=True),
33+
]
34+
35+
type PerDeviceAllocation = dict[DeviceId, AllocationAmount]
36+
"""How much of one slot each individual device contributed."""
37+
38+
type DeviceAllocation = dict[ResourceSlotName, PerDeviceAllocation]
39+
"""The slots one device type served, and the per-device amounts behind each."""
40+
41+
42+
class DeviceCapacityData(BackendAISchema):
43+
mem: int | None = None
44+
proc: int | None = None
45+
46+
47+
class AttachedDeviceData(BackendAISchema):
48+
device_id: DeviceId
49+
model_name: str
50+
data: DeviceCapacityData = Field(default_factory=DeviceCapacityData)
51+
52+
53+
class ServicePortData(BackendAISchema):
54+
name: str
55+
protocol: ServicePortProtocols
56+
container_ports: list[int] = Field(default_factory=list)
57+
host_ports: list[int | None] = Field(default_factory=list)
58+
is_inference: bool = False
59+
60+
61+
class KernelCreationInfo(BackendAISchema):
62+
"""
63+
What the agent reports about a kernel once its container is up.
64+
65+
This is the contract with the manager, not a rendering of the agent's own resource
66+
spec: it carries the facts the manager records against the kernel and nothing else.
67+
"""
68+
69+
container_id: ContainerId
70+
kernel_host: str
71+
repl_in_port: int
72+
repl_out_port: int
73+
service_ports: list[ServicePortData] = Field(default_factory=list)
74+
attached_devices: dict[DeviceName, list[AttachedDeviceData]] = Field(default_factory=dict)
75+
allocations: dict[DeviceName, DeviceAllocation] = Field(default_factory=dict)
76+
77+
def to_resource_slot_entries(self) -> list[ResourceSlotEntry]:
78+
"""
79+
Sum the per-device allocations into the occupancy the manager accounts by.
80+
81+
A slot holding no device allocation is left out rather than reported as zero,
82+
which is the distinction a caller storing the result as occupancy depends on.
83+
"""
84+
entries: list[ResourceSlotEntry] = []
85+
for device_allocation in self.allocations.values():
86+
for slot_name, per_device in device_allocation.items():
87+
if not per_device:
88+
continue
89+
total = sum(per_device.values(), Decimal(0))
90+
entries.append(ResourceSlotEntry(resource_type=slot_name, quantity=str(total)))
91+
return entries

0 commit comments

Comments
 (0)