Skip to content

Batched source sync reports complete before earlier items are recallable #7474

Open

Description

@mysma-9403

Summary

#7253 merged, and with it one behaviour change that was raised in review but never decided: a batched source sync no longer waits for ranked recall on every item, only on the last of each batch. So store_all can return — and run_and_record can mark the source Idle — while earlier items of a batch are listed but not yet recallable.

Problem / Context

Verified in the engine, not inferred:

  • cortex/engine/store.rs::store_items ends with await_written(written, index == final_index) per scope.
  • cortex/log/write.rs::await_written(_, settle) calls await_readable(...) — the ranked-recall wait — only when settle is true, and otherwise just await_listed(...).
  • So exactly one event per batch, the final one, is awaited for ranked recall.
  • The engine's own doc is explicit that monotonicity holds for listing and says nothing of the kind for recall: "the scope's log is ordered, so it being listed implies the earlier ones are), and ranked recall for the batch's final event only."
  • The path Store memory source items in batches, as backfill already does #7253 replaced went store_on → store_on_with(.., WriteOptions::visible()) → a single-item store_items, where that item is the final event. Every item used to be recall-settled.

There is no stronger batch call to switch to. store_many_on already uses WaitFor::Visible; the settle-for-final-event-only is internal to store_items and not selectable through WriteOptions, and the only other value (WaitFor::Accepted) is weaker still. That is why this is a decision rather than a patch.

What is not at stake: which items get stored, which fail, the is_account_wide() early exit, or the touched node set that keys the belief-build jobs. Listing visibility also still holds, so list-shaped reads (memory_list and friends) see every item when sync reports complete. Only ranked recall can trail, bounded by the index refresh, and only for non-final items of a batch.

Scope (optional)

The question is whether "source sync reported complete" must imply "recallable" on this surface. Three answers, and I do not think an outside contributor should pick:

  1. Accept it. Document the trade-off on store_all. import and backfill already live with exactly this, so it would make the three consistent.
  2. Revert this path to per-item. Restores the old guarantee exactly and gives back the batching — N network round-trips instead of ceil(N / 100), since the engine is always an HTTP Cortex engine.
  3. Ask upstream for the option. A per-item — or last-per-scope — settle flag on tinymemory's store_many_with would keep both properties at one request per chunk. This is the only answer that does, and it is not in this repository.

Acceptance criteria

  • Decided — one of the three above is chosen, and the reasoning is written down where the next reader of store_all will find it.
  • If (1) — the trade-off is on store_all's doc comment, not only in a PR description, since that is where it merged without being stated.
  • If (2) or (3) — the change lands with a test that a sync reporting complete implies every item is recallable.
  • Diff coverage ≥ 80% — if the answer involves code.

Related

Raised by review on #7253 (CodeRabbit, Major) and verified against the engine before filing; the PR merged before the question was settled, so it is filed here rather than left in a merged thread. Origin issue: #7252. Related, same domain, already filed: #7366. Follow-up to the same merge: the drain(..n) front-copy in the merged code, fixed separately.

Activity

  1. added
    priority: p2Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.
    on Oct 12, 2026
  2. tinysweeper commented on Oct 12, 2026

    @tinysweeper

    A batched source sync from #7253 now only awaits ranked recall for the final event per batch, so store_all can report complete and the source can be marked Idle while earlier items are listed but not yet recallable. Verified in engine code (store_items settles only index == final_index; await_written skips await_readable otherwise). No stronger batch call exists, so a decision is required among accepting/documented trade-off, reverting to per-item settle, or requesting an upstream settle flag. Listing visibility still holds; failure is bounded by index refresh and affects only non-final items, giving it a workaround or at least a narrow window.

    Labelled priority: p2.

  3. added theissue type on Oct 12, 2026
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

    priority: p2Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions