tarantool / tarantool / 5723

91%

master: 87%

push

travis-ci

iproto: fix IPROTO_MSG_MAX check

We shouldn't allow more than IPROTO_MSG_MAX requests, because this can
result in depleting the tx fiber pool (FIBER_POOL_SIZE) and, as a
result, a deadlock (e.g. WAL needs a fiber to wake up another fiber
waiting for write to complete).

The IPROTO_MSG_MAX check was effectively disabled by commit 52b479518
("Don't stop connection if there is no pending requests"):

> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -590,7 +590,10 @@ iproto_connection_on_input(ev_loop *loop, struct ev_io *watcher,
>       try {
>               /* Ensure we have sufficient space for the next round.  */
>               struct iobuf *iobuf;
> -             if (mempool_count(&iproto_msg_pool) > IPROTO_MSG_MAX ||
> +             if ((mempool_count(&iproto_msg_pool) > IPROTO_MSG_MAX &&
> +                     /* Don't stop connection if there is no pending requests */
> +                     (ibuf_used(&con->iobuf[0]->in) > con->parse_size ||
> +                      ibuf_used(&con->iobuf[1]->in))) ||
>                   (iobuf = iproto_connection_input_iobuf(con)) == NULL) {
>
>                       ev_io_stop(loop, &con->input);
>
>         (ibuf_used(&con->iobuf[0]->in) > con->parse_size ||
>          ibuf_used(&con->iobuf[1]->in))

because

  (ibuf_used(&con->iobuf[0]->in) > con->parse_size ||
   ibuf_used(&con->iobuf[1]->in))

is almost always false, since iproto_enqueue_batch() parses and flushes
all available input so that both iobufs are empty and the parse_size is
typically 0.

It looks like the problem the above-mentioned commit tried to address is
different. The thing is that mempool_count(&iproto_msg_pool) accounts
not only active requests, but also disconnect messages allocated for
each connection in advance. As a result, if there were more than
IPROTO_MSG_MAX (768) connections, iproto would get stuck forever.

That said, the true fix would be subtracting the number of connections
from the number of objects allocated from... (continued)

8 of 8 new or added lines in 1 file covered. (100.0%)

24590 of 32318 relevant lines covered (76.09%)

753022.93 hits per line