Skip to content

[C++] Batch ingest API forces callers to copy every record #724

Description

@zlata-stefanovic-db

Summary

Stream::ingest_proto_records accepts records only as
const std::vector<std::vector<std::uint8_t>>&. Callers whose encoded records
are not already laid out in exactly that container, grouped exactly as they want
to send them, must copy every record into a temporary vector before each call.
That copy is not required for memory safety — the SDK never uses the container
it demands.

The copy is not needed

The C FFI entry point borrows the record bytes as parallel pointer and length
arrays:

int64_t zerobus_stream_ingest_proto_records(struct CZerobusStream *stream,
                                            const uint8_t *const *records,
                                            const uintptr_t *record_lens,
                                            uintptr_t num_records,
                                            struct CResult *result);

make_proto_batch in cpp/src/stream.cpp adapts to it by collecting pointers
and lengths only; it copies no record bytes. The Rust core then makes the one
copy that is genuinely necessary, because the send is asynchronous and it must
own the bytes.

So the lifetime contract is already "the bytes must be valid for the duration of
the call" — it has to be, since the current API hands Rust raw pointers into the
caller's vector and the caller may destroy that vector as soon as the call
returns. A borrowing overload would carry exactly the contract that ships today.

The copy is imposed by the parameter type, not by the memory model.

Who this affects

Any C++ caller whose records live somewhere other than that specific container
shape: a window into a larger pool, an arena, a ring buffer, a memory-mapped
file, or a std::vector<std::string>. The workaround is to duplicate the data
into the shape the API wants, which for large in-memory datasets is not
acceptable. The batch API is also the path CLAUDE.md directs users to prefer in
hot paths, so this is on the recommended route.

Measured impact

Found while comparing the Rust, Go (pure and cgo), and C++ SDKs on identical
payloads. The C++ benchmark worker pays this copy per batch, on top of the Rust
core's copy — two copies per record against Rust's one and pure-Go's zero.

dataset record rate C++ vs fastest SDK, 1 stream
ClickBench, 105 columns ~99k rec/s no measurable gap
synthetic, 3 columns ~243k rec/s ~13% slower

The cost scales with records per second, so it hides inside the server-side
ceiling on wide tables and surfaces on narrow ones. At 8 streams the gap closes,
because the copy is parallelizable client-side work and the server-side ceiling
binds first.

Proposed fix

Add a borrowing overload. Additive, so nothing existing changes:

/// A borrowed view of one protobuf-encoded record. Holds no ownership.
struct ProtoRecord {
  const std::uint8_t* data;
  std::size_t size;
};

std::int64_t ingest_proto_records(const ProtoRecord* records,
                                  std::size_t num_records);

The implementation splits the views into the parallel arrays the FFI wants,
exactly as make_proto_batch does now — pointers and lengths only. The existing
vector overload stays as it is; it should not delegate through the new one, since
that would make it build views and then split them, two passes where it does one
today.

A signature exposing the FFI's parallel arrays directly would be adaptation-free,
but it leaks uintptr_t into the public API to save two small pointer-array
allocations per batch rather than thousands of byte copies. The view struct looks
like the better trade. (std::span would be the natural choice but the project
targets C++17.)

Notes

  • ingest_json_records has the identical gap: it requires
    const std::vector<std::string>& while the FFI takes const char* const*.
  • No change to rust/ffi/zerobus.h, so the Go and Java SDKs are unaffected and
    no FFI release is needed. C++-only and additive, so a minor bump.
  • cpp/tests/ currently contains no reference to ingest_proto_records, so the
    batch path appears to have no test coverage. Worth adding alongside: empty
    batch, null array with a non-zero count, zero-length record, closed stream, and
    agreement between the two overloads.

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