Skip to content

Commit

Permalink
proxy: fix short writes caused by mcp.internal
Browse files Browse the repository at this point in the history
Wasn't filling in the 'tosend' portion of a response structure when
using mcp.internal to create a response object.

This field is used as a fastpath check to see if a response has been
completely transmitted to the socket or not. Since it was zero, we would
always consider an mcp.internal() based response as completely sent,
cutting off the data for larger items.

Expands tests for "simply large" and also a large enough to be chunked
item.
  • Loading branch information
dormando committed May 3, 2024
1 parent 8a188a0 commit a4d8324
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
2 changes: 2 additions & 0 deletions proto_proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -805,8 +805,10 @@ static void _proxy_run_tresp_to_resp(mc_resp *tresp, mc_resp *resp) {
// So far all we fill is the wbuf and some iov's? so just copy
// that + the UDP info?
memcpy(resp->wbuf, tresp->wbuf, tresp->iov[0].iov_len);
resp->tosend = 0;
for (int x = 0; x < tresp->iovcnt; x++) {
resp->iov[x] = tresp->iov[x];
resp->tosend += tresp->iov[x].iov_len;
}
// resp->iov[x].iov_base needs to be updated if it's
// pointing within its wbuf.
Expand Down
2 changes: 1 addition & 1 deletion proxy_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ int mcplib_internal_run(mcp_rcontext_t *rctx) {
// resp object is associated with the
// response object, which is about a
// kilobyte.
lua_gc(rctx->Lc, LUA_GCSTEP, 1);
t->proxy_vm_extra_kb++;

if (resp->io_pending) {
// TODO (v2): here we move the IO from the temporary resp to the top
Expand Down
38 changes: 34 additions & 4 deletions t/proxyinternal.t
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,50 @@ note "ascii basic";
is(scalar <$ps>, "VALUE /b/foo 0 2\r\n", "get response");
is(scalar <$ps>, "hi\r\n", "get value");
is(scalar <$ps>, "END\r\n", "get END");
check_version($ps);
}

#diag "fetch from extstore"
{
subtest 'basic large item' => sub {
my $data = 'x' x 500000;
print $ps "set /b/beeeg 0 0 500000\r\n$data\r\n";
is(scalar <$ps>, "STORED\r\n", "big item stored");

print $ps "get /b/beeeg\r\n";
is(scalar <$ps>, "VALUE /b/beeeg 0 500000\r\n", "got large response");
is(scalar <$ps>, "$data\r\n", "got data portion back");
is(scalar <$ps>, "END\r\n", "saw END");

print $ps "delete /b/beeeg\r\n";
is(scalar <$ps>, "DELETED\r\n");
check_version($ps);
};

subtest 'basic chunked item' => sub {
my $data = 'x' x 900000;
print $ps "set /b/chunked 0 0 900000\r\n$data\r\n";
is(scalar <$ps>, "STORED\r\n", "big item stored");

print $ps "get /b/chunked\r\n";
is(scalar <$ps>, "VALUE /b/chunked 0 900000\r\n", "got large response");
is(scalar <$ps>, "$data\r\n", "got data portion back");
is(scalar <$ps>, "END\r\n", "saw END");

print $ps "delete /b/chunked\r\n";
is(scalar <$ps>, "DELETED\r\n");
check_version($ps);
};

subtest 'fetch from extstore' => sub {
my $data = 'x' x 1000;
print $ps "set /b/ext 0 0 1000\r\n$data\r\n";
is(scalar <$ps>, "STORED\r\n", "int set for extstore");
sleep 3; # TODO: import wait_for_ext
wait_ext_flush($ps);

print $ps "get /b/ext\r\n";
is(scalar <$ps>, "VALUE /b/ext 0 1000\r\n", "get response from extstore");
is(scalar <$ps>, "$data\r\n", "got data from extstore");
is(scalar <$ps>, "END\r\n", "get END");
}
};

#diag "flood memory"
{
Expand Down

0 comments on commit a4d8324

Please sign in to comment.