Skip to content

[doc] Document the contract of storage-side callables (data_parser, merge_fn) with a tutorial #195

Open

Description

@0oshowero0

Background

Three KV APIs run a user callable on the SimpleStorage units:

API Callable What it receives
kv_put / kv_batch_put data_parser(data) -> data The unit's share of the new batch, before it is stored. A regular tensor column arrives as one batched tensor; nested-tensor and NonTensorStack columns arrive as lists.
kv_batch_get (#191) data_parser(data, args) -> data The stored values, as one list per column with one value per sample (tensor columns included), plus the matching parser_args entries.
kv_update / kv_batch_update (#182) merge_fn(old, new) -> stored One sample's stored value and its new value, per field.

They are powerful but easy to misuse, and their contracts are documented unevenly, in places inaccurately.

Gaps in the current docs

  1. put data_parser: "must modify in-place" is inaccurate. The docstrings of kv_put and kv_batch_put say the parser must modify values in place, but the unit requires it to return a dict and raises TypeError otherwise (_apply_data_parser). Modifying in place is allowed, not required.
  2. The get and put data_parser look interchangeable but are not. The get docstring calls itself the get-side counterpart of put's parser, but the two differ in arity ((data, args) vs (data)) and in input structure (per-sample lists vs a batched tensor for regular tensor columns). Passing a put parser to kv_batch_get fails on the unit with takes 1 positional argument but 2 were given.
  3. merge_fn does not say that old is the stored object. apply_update passes the stored value itself (L255). If a merge_fn modifies old in place, the guarantee that a failed merge leaves storage unchanged no longer holds: samples merged before the failure have already changed.
  4. Runtime constraints are not stated in one place. All three run synchronously on the unit's single worker thread, so a slow one delays every other request to that unit. A call that uses one of them is not retried on a storage-unit timeout, and a timeout has an unknown outcome. Callables travel through cloudpickle, so module-level functions must be importable on the units, and tuples inside parser_args arrive as lists.

Expected work

  • Docs. One shared contract for storage-side callables in the README, referenced from the docstrings of kv_put, kv_batch_put, kv_batch_get, kv_update, kv_batch_update and their async variants: where it runs, the input structure per API, no in-place modification of stored values, no side effects, keep it light, no retry, serialization, and how errors reach the caller. Fix gaps 1-3 above.
  • Dedicated tutorial. A new tutorial/07_*.py that walks through put data_parser (resolving references into content), get data_parser with parser_args (trimming a tensor field and picking items from a list field in one get), and kv_update with merge_fn (appending to a field). Include one in-place example and show why it corrupts stored data.
  • Notebook demo. Extend section 11 of tutorial/basic.ipynb ("Lazy Data Parsing with data_parser") with a get-side data_parser demo, and add a kv_update / merge_fn demo.

Related: #182, #191

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions