Skip to content

perf(orchestrator): coalesce NBD header + max payload in one read - #3373

Open
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/nbd-dispatch-buffer-header-size
Open

perf(orchestrator): coalesce NBD header + max payload in one read#3373
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/nbd-dispatch-buffer-header-size

Conversation

@AdaAibaby

@AdaAibaby AdaAibaby commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

fix #3355

Problem

dispatchBufferSize was exactly 4 MiB — the maximum typical data payload size. Every NBD request also carries a 28-byte header, so a max-size (4 MiB) write required two Read syscalls instead of one:

First Read:  [28B header] [4MiB - 28B payload]   ← fills 4 MiB buffer exactly
Second Read: [remaining 28B of payload]

The code had a TODO acknowledging this:

// TODO: Look into optimizing the buffer reads by increasing the buffer size by 28 bytes,
// to account for a request that is 28 bytes of header + 4MB of data.
dispatchBufferSize = 4 * 1024 * 1024

Fix

  • Add nbdRequestHeaderSize = 28 named constant — replaces all bare 28 literals in the dispatch loop, making the header-size relationship explicit.
  • Increase dispatchBufferSize to 4*1024*1024 + nbdRequestHeaderSize.
  • Remove the resolved TODO.
  • Simplify the now-inaccurate inline comment in the write path.

With the new buffer size, [28B header + 4MiB payload] fits in a single Read call, saving one syscall per max-size request. For I/O-heavy sandboxes doing large sequential block writes, this adds up across thousands of requests.

Memory impact: 28 bytes per sandbox connection — negligible at any scale.

Tests

  • TestDispatchMaxSizeWrite — sends a 4 MiB write (header + full payload as one chunk) and verifies the payload reaches the backend with correct offset and full data integrity.
  • nbdRequest() helper updated to use nbdRequestHeaderSize instead of a bare 28.

/cc @jakubno @dobrac @ValentaTomas @arkamar @tvi Looking forward to your code review.

Add nbdRequestHeaderSize = 28 constant and increase dispatchBufferSize
from 4 MiB to 4 MiB + 28 bytes.

An NBD request is [28-byte header][payload]. With a 4 MiB buffer a
max-size 4 MiB payload arrives in two Read calls: the first fills the
buffer with [28B header + (4MiB - 28B) payload], and a second call
fetches the remaining 28 bytes. Adding 28 bytes to the buffer lets the
entire [28B header + 4MiB payload] fit in a single Read, saving one
syscall per max-size request.

Memory impact: 28 bytes per sandbox connection — negligible.

Also replace all bare 28 literals in Handle() with nbdRequestHeaderSize,
and remove the now-resolved TODO comment.

Fixes e2b-dev#3355
@cla-bot cla-bot Bot added the cla-signed label Jul 24, 2026
@AdaAibaby
AdaAibaby force-pushed the fix/nbd-dispatch-buffer-header-size branch from 41bb733 to f729c65 Compare July 25, 2026 07:57
@ValentaTomas
ValentaTomas force-pushed the main branch 2 times, most recently from 5aad415 to d71980e Compare July 25, 2026 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(orchestrator): increase NBD dispatch buffer by 28 bytes to coalesce header + max payload in one read

2 participants