Repository navigation
[feat] add the __public__ partition and kv link api to hold the shared fields and share them in different trajectories. - #200
OutstanderWang wants to merge 38 commits into
Conversation
GRPO-style rollouts put one prompt and link its n responses to it, so the prompt payload is stored once. Links are controller metadata only; the SimpleStorage read path resolves them and reads each source row once. A put on a linked field replaces the link, clearing a linked key drops only the link, and a source cannot be cleared while other keys still link to it. dump_data_by_key records links and load_data_by_key relinks. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
kv_link checked field metadata, which stays until the clear finishes, so a link made between mark_clearing and the storage delete pointed at a payload about to vanish. Check production status, which mark_clearing zeroes first. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
A repeated missing key was registered twice, leaving a keyless row that held a link and could never be cleared. Reject duplicates before any RPC, as the batch writes do. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…lear Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…elds Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…link Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
… ready again Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…uard Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5837879910
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <dump_dir>/ | ||
| dump_info.json # version, partition, counts | ||
| row_index.pt # key -> {global_index, fields, unfinalized, tag} | ||
| row_index.pt # key -> {global_index, fields, unfinalized, links, tag} |
There was a problem hiding this comment.
Bump the dump format before writing links
When a link-bearing dump is restored by the pre-change build, dump_info.json still advertises format 4, which that reader accepts. That reader ignores the new links member while fields deliberately omits linked payloads, so the restore succeeds with those fields silently lost. Allocate a new dump format version (while retaining v3/v4 read compatibility) so older readers reject link-aware dumps.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: main's reader loads a link-bearing dump without error and the linked field is gone. Fixed in 8fdf3ff: dumps are written as format 5, and this build still reads 3 and 4 (a test rewrites a fresh dump as each older version and restores it).
| fields_by_row: dict[int, set[str]] = defaultdict(set) | ||
| for global_index, links in zip(metadata.global_indexes, metadata._links, strict=True): | ||
| for field in metadata.field_names: | ||
| fields_by_row[links.get(field, global_index)].add(field) |
There was a problem hiding this comment.
Preserve duplicate samples through parser grouping
When kv_batch_get receives repeated keys/global indexes together with data_parser and per-key parser_args, this dictionary coalesces occurrences before routing. The parser runs only once with one occurrence's argument and the reordered output duplicates that result, so another duplicate ignores its own argument; the previous routing path kept duplicate positions. Preserve occurrences for parser-backed reads or reject duplicate read keys.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and it was wider than duplicates: batch_positions indexed the deduplicated row list, so every key after a repeat got its neighbour's argument ([a, a, b, c] with [1, 2, 3, 4] read [11, 11, 22, 33]; main reads [11, 12, 23, 34]). Fixed in 4c441ca: a sample's own fields are read once per occurrence, and only linked public rows are deduplicated. Covered in test_kv_interface_e2e.py.
| metadata = self.kv_retrieve_meta(keys, partition_id, create=True) | ||
| partition = self._get_partition(partition_id) | ||
| assert partition is not None | ||
| partition.link_fields(metadata.global_indexes, public, source, fields) |
There was a problem hiding this comment.
Delete private cells when linking existing fields
When a destination key already stores any requested field, kv_link only changes controller link maps; no client or storage-manager call removes the destination cell. Reads switch to the public source, but the old tensor or SSD file remains allocated until an explicit overwrite or clear, so relinking an existing dataset retains all N copies and can still exhaust memory. Reject already-produced destinations or clear their private cells through the data plane when linking.
AGENTS.md reference: AGENTS.md:L13-L14
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Fixed in 78134b5 by refusing instead of deleting through the data plane: kv_link raises when a key already stores one of the fields itself, before creating any key, and asks for kv_clear(fields=...) first.
| await self._request_controller( | ||
| socket=socket, | ||
| request_type=ZMQRequestType.KV_LINK, | ||
| response_type=ZMQRequestType.KV_LINK_RESPONSE, | ||
| body={"keys": keys, "partition_id": partition_id, "public_key": public_key, "fields": fields}, |
There was a problem hiding this comment.
Enforce the backend check in the low-level link client
Under Mooncake or YuanRong, callers of the exported get_client().kv_link/TransferQueueClient.kv_link reach this RPC directly and bypass the backend guard that exists only in interface.async_kv_link. The controller returns success and marks fields linked, but those storage managers do not resolve BatchMeta._links, so subsequent reads use missing or stale destination cells. Put the backend check in this low-level client path as well.
AGENTS.md reference: AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Fixed in ae87d25: the SimpleStorage check moved into TransferQueueClient.async_kv_link, which the sync client and tq.kv_link both go through; the interface-level copy is gone.
| keys = [keys] if isinstance(keys, str) else keys | ||
| _check_unique_keys(keys) |
There was a problem hiding this comment.
Reject empty destination lists before recording links
kv_link([], ...) is accepted here: the controller creates a destination partition and link_fields records linked_by[source] = set() despite having no rows. mark_unlinked_public_clearing considers the reverse-map keys rather than nonempty sets, so that public source is skipped by every bulk-clear call until the empty destination partition is removed. Reject the empty list before the RPC or avoid creating an empty reverse entry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed (kv_link([]) left an empty reverse entry and kv_clear_unlinked_public() returned []). Fixed in 5162c4f: the controller's kv_link rejects an empty key list, so direct client calls are covered too.
| key | ||
| for row in rows.values() | ||
| for name, key in row.get("links", {}).items() | ||
| if name not in public_rows[key]["fields"] or name in public_rows[key]["unfinalized"] |
There was a problem hiding this comment.
Validate linked field types before loading payloads
When loading a linked dump into an existing partition that declares the linked field with an incompatible dtype or tensor kind, this preflight checks only public readiness because linked fields are absent from row_index["field_schema"]. load_rows_by_key then registers keys and may write shards, and the subsequent client.kv_link is the first type validation and fails, leaving a partial restore. Validate the destination against the public fields' schemas before calling load_rows_by_key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed (the payloads of the linking keys were written before kv_link failed). Fixed in b0d2acb: load relinks first. kv_link checks field types before creating any key, so an incompatible destination now fails with nothing written; there is a test for it.
Chase-Rong
Chase-Rong
left a comment
There was a problem hiding this comment.
[AI comments] Links reference a mutable key name, not a snapshot. A linked public row can still be rewritten, and L1271-L1279 propagates the new value and readiness to every linking key. If a later step reuses a public key name (video_0) while older trajectories still link it, they silently read the new payload.
Dump/load has the same gap. Repro: link 4 keys to v0 (values 0..5), dump both, clear them, kv_put("v0", <all 9s>), then load_data_by_key(link_dir). The load succeeds and the restored keys read [9, 9, ...]. The preflight only checks that the public field is ready.
Suggestion: refuse writes to a public row while it is linked, symmetric to the clear guard; this also removes the need for the cascade loop. Record a fingerprint or dump id of the public rows in the link dump so load can verify it, or offer an option to dereference links at dump time.
| assert partition is not None | ||
| partition.link_fields(metadata.global_indexes, public, source, fields) | ||
|
|
||
| def mark_unlinked_public_clearing(self) -> tuple[BatchMeta, list[str]]: |
There was a problem hiding this comment.
kv_clear_unlinked_public() sweeps every unlinked key, including keys another producer has kv_put but not yet kv_linked. The documented precondition ("no writer between kv_put and kv_link") rarely holds in async/off-policy GRPO, where the rollout of step k+1 overlaps the trainer cleanup of step k. The producer kv_link then fails ("must have produced" / "not found") and its payload is gone. __public__ is also one global namespace, so two jobs sharing a TransferQueue can collide on key names and sweep each other's payloads.
There was a problem hiding this comment.
Agreed, the precondition was too strong for overlapping steps. 2e812ec: the controller records which public rows have ever been linked, and kv_clear_unlinked_public frees only those that no key links to any more. A key that is put but not linked yet is never swept, so a producer between kv_put and kv_link is safe; a payload whose producer died before linking needs an explicit kv_clear. The record survives controller checkpoints. For the shared namespace, the docstring now says to prefix public keys with a job id.
| tq_client._run_coroutine(async_kv_clear(keys=keys, partition_id=partition_id, fields=fields)) | ||
|
|
||
|
|
||
| def kv_link(keys: list[str] | str, partition_id: str, public_key: str, fields: list[str] | str) -> None: |
There was a problem hiding this comment.
Each call to kv_link requires a public_key, so processing B prompts requires B RPC calls, and the entire process is not atomic: if a failure occurs midway, it will result in an earlier set of prompts being linked while a later set remains unlinked. I suggest adding an optional batch processing feature, eg. accept public_keys: list[str] aligned with keys or a {key: public_key} mapping
There was a problem hiding this comment.
Done in 1166b53: public_key also accepts a list aligned with keys. The controller checks every public key, field and destination before creating or linking anything, so a batch is linked by one atomic request. load_data_by_key uses it too, with one call per linked field set.
|
[AI comments] A plain read-modify-write silently turns every link into a private copy. Repro: Suggestion: refuse a put that lands on a linked field and require an explicit unlink ( |
|
[AI comments] Suggestion: keep |
| raise KeyError(f"key {public_key!r} not found in partition {PUBLIC_PARTITION!r}") | ||
| source = public.keys_mapping[public_key] | ||
| # Production status, not field metadata: mark_clearing zeroes it before storage deletes the payload. | ||
| missing = [ | ||
| name | ||
| for name in fields | ||
| if name not in public.field_name_mapping | ||
| or public.production_status[source, public.field_name_mapping[name]] != 1 | ||
| ] | ||
| if not fields or missing: | ||
| raise ValueError(f"public key {public_key!r} must have produced fields {missing or fields}") |
There was a problem hiding this comment.
The PR description says kv_link refuses keys that are being cleared, but only the public source's production status is checked. I conduct a simple test: kv_put("t0", P), mark_clearing([t0]), then kv_link("t0", P, "v0", "video") is accepted, and video reads as ready during the clear window. The following clear_meta drops the link, so the state converges and the impact is small. A destination being cleared cannot be told apart from one not produced yet by production status alone, so correcting the description is likely simpler than adding the check.
There was a problem hiding this comment.
Confirmed with the same repro. I agree that correcting the description is simpler. The PR description now says a destination being cleared is accepted and that the clear then drops the link.
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…ites nothing Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…link Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
… name the linking keys Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…ed before Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…omic call Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
|
Re: #200 (review) Agreed on the write side. bba58db makes a linked public key read-only: I kept the propagation loop. A put through a For dump/load, links stay by name, and the docstring and |
|
Re: #200 (comment) Confirmed. Fixed in 11d6a38: a put whose |
|
Re: #200 (comment) Confirmed on both points. 514ee42: |
A put of a non-tensor value turns the partition's field non-tensor, and a later put of a tensor still lands. kv_link checked the strict restore schema instead, so linking a tensor prompt into a field that had held a text-only prompt failed. Reject only links that the merge itself would fail on: two tensor dtypes. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…ked_public frees stale never-linked keys kv_exists returns the ready fields of each existing key and omits missing keys, so a caller can probe for a key without the controller's not-found warnings. kv_clear_unlinked_public(stale_keys=...) also frees the listed public keys that no key ever linked, for producers whose every consumer went away. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
… like keys - TransferQueueClient.link / async_link(metadata, public_metadata, fields) links rows by global index and returns a BatchMeta whose _links point at the public rows. - A BatchMeta put into PUBLIC_PARTITION is refused while other rows link to the target rows, so a linked public row is read-only on both write paths. - kv_clear_unlinked_public sweeps public rows by index, so rows without keys are freed too. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
…c row's storage unit A linked field used to refuse a data_parser. It is now read from its public row once per distinct plain parser_args entry (scalars and lists of them) among the keys linking to it, and the public row's storage unit parses it, so only the parsed part is sent back. Any other parser_args entry, such as a tensor, is parsed once per key. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
b19121c to
a22f20b
Compare
0oshowero0
left a comment
There was a problem hiding this comment.
[AI Review]
Two reproducible P2 findings in the BatchMeta link and linked-field parser paths.
| if field in links: | ||
| linked = (links[field], field, _parser_arg_identity(parser_arg, pos)) | ||
| if linked not in linked_slots: | ||
| linked_slots[linked] = ((field,), len(rows_by_fields[field,])) |
There was a problem hiding this comment.
[AI Review] [P2] Preserve joint parser input for fields on the same public source row and document cross-source limitations
This suggestion applies only when multiple selected fields link to the same public source row. They have the same actual global index and reside on the same StorageUnit, so one GET can read them together and pass them to the parser.
The current implementation still splits these fields into single-field requests. As a result, a parser that works when reading the public record directly fails when reading the same fields through links.
For example, public record video_0 stores both a padded video and its valid length. Trajectory traj_0 links both fields to video_0:
import torch
import transfer_queue as tq
# Assume TransferQueue has been initialized with SimpleStorage.
tq.kv_put(
key="video_0",
partition_id=tq.PUBLIC_PARTITION,
fields={
"video": torch.tensor([10, 20, 0, 0]),
"length": torch.tensor([2]),
},
)
tq.kv_put(
key="traj_0",
partition_id="train",
fields={"response": torch.tensor([99])},
)
tq.kv_link(
keys=["traj_0"],
partition_id="train",
public_key="video_0",
fields=["video", "length"],
)
def crop_video(data, args):
return {
"video": [
video[:int(length.item())]
for video, length in zip(data["video"], data["length"])
],
"length": data["length"],
}
# Reading the public record directly succeeds: video becomes [10, 20].
direct = tq.kv_batch_get(
keys=["video_0"],
partition_id=tq.PUBLIC_PARTITION,
select_fields=["video", "length"],
data_parser=crop_video,
)
# Reading through links fails with RuntimeError reporting a missing field.
via_link = tq.kv_batch_get(
keys=["traj_0"],
partition_id="train",
select_fields=["video", "length"],
data_parser=crop_video,
)The direct read sends one request:
source row: the public row for video_0
fields: ["video", "length"]
The linked read instead produces:
request 1: the same public source row, fields=["video"]
request 2: the same public source row, fields=["length"]
The parser therefore receives only video or only length in each invocation and raises KeyError inside the StorageUnit when accessing the other field. The SDK exposes this as a RuntimeError reporting the missing-field error.
Please group fields from the same public source row with the same parser-argument identity into one read request, and add a regression test that executes the cross-field parser above. This grouping should retain the existing _parser_arg_identity and deduplication policy, without introducing value comparisons for tensor arguments. Reading and parsing still happen on the source StorageUnit; this does not require combining data across storage locations.
For reads that mix public and non-public fields, please explicitly document the limitation in the kv_batch_get and get_data / async_get_data docstrings:
data_parserexecutes per storage read group. Each invocation receives only the fields in its current group. When selected fields of one logical sample come from a public source row and a regular row, they may be parsed in separate requests. Callers must not assume that one parser invocation receives all selected fields of that logical sample, or depend on fields from different source rows within that sample being joined for parsing.
This comment does not request joining fields that resolve to different source rows or storage locations within a single logical sample. A parser invocation may still process multiple source rows as a batch. Even when all fields belong to the public partition, joining fields from different source rows within one logical sample is outside the scope of this suggestion.
| "data_fields": list(dict.fromkeys([*metadata.field_names, *fields])), | ||
| }, | ||
| ) | ||
| return response.body["metadata"] |
There was a problem hiding this comment.
[AI Review] [P2] Preserve caller-local metadata when returning linked BatchMeta
The new BatchMeta returned by link() does not retain the input batch's extra_info or local custom_meta changes not yet synchronized to the controller. The original input object remains unchanged, and this does not delete tags already persisted in the controller. The concern is the information missing from the returned object used for subsequent processing or writes.
Repro with one destination row whose persisted custom metadata is empty, and one public row:
rows_meta.extra_info["epoch"] = 3
rows_meta.update_custom_meta([{"group": "g0"}])
linked = client.link(rows_meta, public_meta, "prompt")
assert linked.extra_info == rows_meta.extra_info # fails: {} vs {"epoch": 3}
assert linked.custom_meta == rows_meta.custom_meta # [{}] vs [{"group": "g0"}]The returned metadata is missing both values, although they remain present in rows_meta. This can also remove per-sample information used to construct get-parser arguments.
Please distinguish the handling of these two kinds of metadata:
- Preserve the caller's
extra_infoin the returned batch. - Define how unsynchronized local
custom_metais reconciled with the controller's current custom metadata, including precedence when both contain a value for the same key. Simply copying the old input metadata over the controller response could overwrite newer controller values. The controller's updated links, field schema, and readiness should remain authoritative.
This concerns metadata carried by the returned object; it does not require link() to persist local tags automatically. Please cover extra_info and unsynchronized custom_meta in regression tests, and make any conflict-handling policy explicit.
|
[AI Review] Suggestion: document the write-back contract for linked fields A concrete GRPO-style case: one prompt video is stored once in linked = client.link(
rows_meta, public_meta.select_samples([0] * n), "video"
)
batch = client.get_data(linked) # contains both video and response
batch["response"] = updated_responses
write_meta = linked.select_fields(["response"])
client.put(batch, metadata=write_meta)However, For now, I suggest adding a
This observation concerns linked destination fields in a BatchMeta-based put. The guard against overwriting the public source row is a separate controller check. A safe-write example would make the intended usage clear: batch = client.get_data(linked)
batch["response"] = updated_responses
client.put(batch.select("response"), metadata=linked)A short note in |
|
[AI Review] Suggestion: document that a public-row overwrite must not overlap with linking that row For now, I suggest adding a caller-side concurrency warning to the
The following SDK-level case explains the restriction. Initial stateAssume TransferQueue is initialized with SimpleStorage. Store one public video and one trajectory response: import torch
from tensordict import TensorDict
import transfer_queue as tq
client = tq.get_client()
public_meta = client.put(
TensorDict({"video": torch.tensor([[1, 2, 3]])}, batch_size=1),
partition_id=tq.PUBLIC_PARTITION,
)
traj_meta = client.put(
TensorDict({"response": torch.tensor([[0]])}, batch_size=1),
partition_id="train",
)Call the allocated public row Pass the metadata to two workers connected to the same TransferQueue. The following calls overlap; the snippets alone do not force a particular scheduling order. Worker A: overwrite the existing public rowclient_a = tq.get_client()
def prepare(data):
return {name: values + 10 for name, values in data.items()}
client_a.put(
TensorDict({"video": torch.tensor([[7, 8, 9]])}, batch_size=1),
metadata=public_meta,
data_parser=prepare,
)Supplying Worker B: establish a link and read through itclient_b = tq.get_client()
linked = client_b.link(traj_meta, public_meta, "video")
first = client_b.get_data(linked)
# Further processing occurs before the next read.
second = client_b.get_data(linked)One permitted interleaving
The deterministic reproduction used two asynchronous SDK operations sharing one client in the same process, with events pausing A after its controller check and before its storage-manager put, allowing B to link and read, then resuming A. The two-worker SDK snippets above illustrate the corresponding possible interleaving; they do not force it by themselves. The scheduling hook only fixes the interleaving; parsing and payload reads/writes still use real SimpleStorage units. The same concurrency restriction applies without a put parser; this parser simply makes the replacement value easy to distinguish. A write-once public-row workflow, with the initial put completed before linking, avoids this overwrite/link overlap. Please make the same-row serialization requirement explicit in the docstrings so callers do not interpret the current write check as protection against concurrent link creation. |
… caller metadata - get_data reads the fields a row links to one public row in one unit request, so a data_parser sees all of them together, as it does when reading the public row itself. - TransferQueueClient.link returns the controller's links and schema with the caller's extra_info and custom_meta, which the controller does not hold. - put / async_put, async_link and BatchMeta.select_fields document that the written fields are data.keys(), that select_fields filters only local metadata, and that a public-row overwrite must not overlap with linking that row. Signed-off-by: OutstanderWang <wangweiyanster@gmail.com>
Problem
Workloads that read one large payload from many keys store it once per key. In GRPO, for example, each prompt video is read by its
nresponses, so putting the video into every response key multiplies its memory and put traffic byn.The same workloads also suffer from unbalanced storage units. SimpleStorage stores
global_indexon unitglobal_index % num_data_storage_units, and indexes were handed out globally in sequence. When puts alternate between two partitions on two units, all four rows of one partition land on unit 0 ([0, 0, 0, 0]). Large shared payloads make this imbalance much worse.Change
1. Balance each partition's rows across SimpleStorage units
PartitionIndexManagercounts each partition's rows per storage unit. Each new index goes to the unit where that partition has the fewest rows. On a tie it prefers a reusable index, then the counter's next index. The example above now spreads as[0, 0, 1, 1].SimpleStorage.num_data_storage_unitsinstore_config. Other backends, and SimpleStorage with one unit, keep the previous FIFO allocation.reusable_indexeslist, which is regrouped by unit on load.2. Share payloads through
__public__andkv_linktq.PUBLIC_PARTITION("__public__") is an ordinary partition that holds shared payloads. It is written and read with the usual KV APIs, and change 1 balances its rows like any other partition's.tq.kv_link(keys, partition_id, public_key, fields)makesfieldsofkeysread the payload ofpublic_keywithout copying it.public_keyis one key for allkeys, or a list aligned withkeysto link a batch of prompts at once. Missing keys are created. The link is a single controller request that checks everything first: either every key is linked or none is, and a refused call creates no key.kv_put,kv_updateand clears on it are refused until no key links to it, so linking keys never see their shared payload change.tq.kv_clear_unlinked_public(stale_keys=None)clears every public key that keys linked to before but none links to now, and returns the cleared keys. A public key that was put but never linked is left alone, so a producer between itskv_putandkv_linkis safe. Pass such keys instale_keysonce their producer is given up on, and they are cleared too if nothing links to them.tq.kv_exists(keys, partition_id)maps each existing key to its ready fields, linked ones included, and leaves missing keys out without logging. It is the probe for "has another worker already put this shared prompt?", wherekv_describewould raise andkv_batch_getwould warn.3. Link, write and clear through
BatchMetaRows without keys can share payloads too.
TransferQueueClient.link(metadata, public_metadata, fields)(andasync_link) links the rows ofmetadatato the aligned rows ofpublic_metadataand returns aBatchMetawhose_linkspoint at the public rows. It refuses whatkv_linkrefuses: linking into__public__, misaligned rows, a public field not produced yet, and rows that already store the field.BatchMetainto__public__asks the controller first and is refused while other rows link to the target rows, so a linked public row is read-only on both write paths.kv_clear_unlinked_publicsweeps public rows by index, so keyless rows are freed as well. It still returns only the keys it cleared.4.
data_parseron linked fieldskv_batch_get(..., data_parser=..., parser_args=...)used to refuse linked fields. A linked field is now parsed on the storage unit that holds its public row, so only the parsed part crosses the network, e.g. the frames each trajectory needs instead of the whole video. The public row is read and parsed once per distinctparser_argsentry among the keys linking to it: keys with equal plain entries (scalars, and lists or tuples of them, compared by type and value) share one parse, and any other entry, such as a tensor, is parsed once per key. Withoutparser_args, each public row is parsed once.How links are implemented:
DataPartitionStatus.links({global_index: {field: public global_index}}) and indexes them by source inlinked_by, so clear checks scale with the size of the clear.BatchMetacarries the links to the storage manager.AsyncSimpleStorageManager.get_datareads each linked public row once, however many keys in the batch link to it, or withdata_parseronce per distinctparser_argsentry.How other APIs behave with links
kv_batch_get,get_data, samplers,StreamingDataLoaderkv_put/ put throughBatchMetaon a linked fieldkv_clear(fields=...)first to store a private valuekv_updateon a linked fieldkv_put/kv_update/ put throughBatchMetaon a linked public key or rowkv_clearof a linking key or fieldkv_clear,clear_samples,clear_partitionon a public key that is still linkedkv_linkon a field the key already stores itselfkv_clear(fields=...)first, so no unread private copy stays in storagekv_describefieldslists every readable field, linked ones included;unfinalizedits not-ready subset;linksmaps each linked field to its public keydump_data_by_key/load_data_by_keysave_checkpoint/load_checkpointdata_parserparser_argsentrykv_existskv_link,TransferQueueClient.kv_linkandTransferQueueClient.linkraiseNotImplementedErrorConcurrency
kv_linkrefuses a public key that is being cleared (mark_clearinghas zeroed its production status). A destination key being cleared cannot be told apart from one not produced yet, so a link to it is accepted; the clear then drops that link.kv_clear_unlinked_publicselects and marks the rows in one controller request, so a racingkv_linkis refused for the keys it clears.kv_putduring a public key's clear can make the key ready again and let akv_linkthrough. When the clear then releases the row, the controller drops that link, so the field becomes unwritten instead of pointing at an index that another partition may reuse.kv_linkvalidates field types before it creates keys, so a dtype conflict leaves no empty keys behind.Limitations
__public__is shared by every job on one TransferQueue; prefix public keys with a job id.kv_updatecannot modify linked fields.kv_batch_getcall only; nothing is cached across calls. Keys sharing a parse may receive the same Python object for a non-tensor field, so modifying one row in place changes the others. As before, adata_parsermust not modify the stored values it receives: one public row requested with differentparser_argshands the same stored object to each parse.Tests
tests/e2e/test_kv_link_e2e.py(new) covers:kv_link;kv_update;kv_describeon linked fields;client.linkreads, its refusals, the read-only guard onBatchMetaputs into__public__, and keyless public rows freed bykv_clear_unlinked_public;data_parserslicing and selecting elements of a linked field, with differentparser_argsper key;kv_clear_unlinked_publicleaving never-linked keys alone and clearingstale_keys;kv_existson present, missing, linked and being-cleared keys;tests/e2e/test_kv_interface_e2e.py: repeated keys keep their ownparser_args.tests/e2e/test_data_dump_e2e.py: version 3 and 4 dumps still load.tests/test_controller_data_partitions.py:tests/test_metadata.py:add_fieldsandunionhandle links.tests/test_async_simple_storage_manager.py: a linked field is requested from its public row's unit once per distinctparser_argsentry, and which entries count as equal.python -m pytest -q tests: 899 passed, 12 skipped. The 8 tests intests/test_yuanrong_storage_client_e2e.pyerror locally because they need a Yuanrong environment; they error the same way without this PR.