Skip to content

Stream all available blobs in DownloadBlobs and skip missing#6201

Merged
ndr-ds merged 1 commit into
mainfrom
ndr-ds/fix-download-blobs-server-semantics
May 16, 2026
Merged

Stream all available blobs in DownloadBlobs and skip missing#6201
ndr-ds merged 1 commit into
mainfrom
ndr-ds/fix-download-blobs-server-semantics

Conversation

@ndr-ds

@ndr-ds ndr-ds commented May 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

The streaming DownloadBlobs RPC added in #5976 terminates the response
stream on the first missing blob, dropping any blobs that would have
been yielded after it:

  • gRPC (linera-service/src/proxy/grpc.rs): the handler yields
    Err(Status::not_found(...)) for a missing blob, which terminates
    the gRPC response stream. Subsequent blobs queued for that batch are
    silently lost.
  • Simple transport (linera-service/src/proxy/main.rs): the handler
    interleaves RpcMessage::Error(BlobsNotFound([single_id])) items
    among DownloadBlobResponse items. The current consumer in
    RemoteNode::download_blobs propagates the first error via ?,
    which drops the rest of the batch on the floor.

A batch of N blobs where blob K is missing returns at most K successful
blobs to the caller, even if the validator has all of blobs K+1..N
available. The follow-up #6157 wires this RPC into the per-peer
fallback path and relies on partial progress being reported correctly,
which the current broken server semantics prevent.

Proposal

Both server handlers now yield only the blobs that are present, in
request order, and silently skip missing blobs. The client compares
the IDs it received against the IDs it asked for and re-requests any
missing IDs from another peer.

This preserves the existing wire format on both transports.
Storage-level errors that occur before any blob has been yielded
continue to surface as a top-level error on the response (gRPC) or as
an early RpcMessage::Error (simple transport).

Also addresses the review comment from afck on #6105 / #6155: the
type annotation on created_blob_ids in
Client::process_certificates moves from the let binding to the
.collect() call (turbofish style).

Test Plan

CI.

Release Plan

@ndr-ds ndr-ds force-pushed the ndr-ds/fix-download-blobs-server-semantics branch 2 times, most recently from b49cd8b to 19252f9 Compare May 2, 2026 03:29
@ndr-ds ndr-ds marked this pull request as ready for review May 2, 2026 03:31
Comment thread linera-core/src/client/mod.rs Outdated
.flat_map(|certificate| certificate.value().block().created_blob_ids())
.collect();
.collect::<BTreeSet<BlobId>>();
let required_blob_ids: Vec<_> = certificates

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Here's another type annotation.)

@ndr-ds ndr-ds force-pushed the ndr-ds/fix-download-blobs-server-semantics branch from 19252f9 to 039a360 Compare May 7, 2026 22:46
@ndr-ds ndr-ds enabled auto-merge May 15, 2026 18:08
@ndr-ds ndr-ds disabled auto-merge May 15, 2026 18:08
@ndr-ds ndr-ds force-pushed the ndr-ds/fix-download-blobs-server-semantics branch from 039a360 to 8648b60 Compare May 15, 2026 18:29
@ndr-ds ndr-ds added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit 593b630 May 16, 2026
39 checks passed
@ndr-ds ndr-ds deleted the ndr-ds/fix-download-blobs-server-semantics branch May 16, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants