Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Ilya Kosarev <i.kosarev@tarantool.org>, alyapunov@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v7 3/3] iproto: move greeting from tx thread to iproto
Date: Mon, 21 Dec 2020 17:20:17 +0100	[thread overview]
Message-ID: <d1eb7c2a-7afa-c2d9-9069-8a761d6aba86@tarantool.org> (raw)
In-Reply-To: <2152c51f6222f2d3d9d412183063c8646872ff68.1608373787.git.i.kosarev@tarantool.org>

Hi! Thanks for the fixes!

Unfortunately, it seems the patch does not help. Probably it did earlier,
but now the descriptors are not closed.

The example you use in the ticket is not really hard enough - you
use only one connection, and you treat 'fetch_schema' as success.
Also you assume the connection won't be re-created repeatedly by the
external code.

To test it more thoroughly I used these scripts:

Instance 1:

	box.cfg({listen = 3301})
	box.schema.user.grant('guest', 'super')
	while true do os.execute("sleep 1000") end

I used big sleep so as the instance wouldn't have a chance to handle
the pending connection requests.

Instance 2:

	fiber = require('fiber')
	netbox = require('net.box')
	host = 'localhost:3301'
	count = 0

	function many_connect()
	    while true do
	        local c = netbox.connect(host, {wait_connected = false})
	        while c.state ~= 'fetch_schema' do fiber.yield() end
	        count = count + 1
	        c:close()
	    end
	end

	f1 = fiber.new(many_connect)
	f2 = fiber.new(many_connect)
	f3 = fiber.new(many_connect)
	f4 = fiber.new(many_connect)

I create 4 fibers to reach the descriptor limit faster. You
can try more or less, but with big enough sleep() timeout the
result will be the same. On my machine with 4 fibers I hit the
descriptor limit (2560 in my case) almost immediately.

Each fiber does a totally normal thing - creates a connection and
closes it. Lets assume, they also tried to execute some request,
it failed a timeout, and they decide to re-create the connection.

If the descriptors would be closed on Instance 1, these fibers
would work fine, and 'count' would grow until Instance 1 stops
the sleep(). Or until it will hit OOM due to too many heavy
struct iproto_connection objects, which is not addressed here, but
we agreed to do it separately, so it is ok for now maybe.

But what happens is that it prints

	SystemError accept, called on fd 16, aka [::]:3301: Too many open files

And no new connections can be accepted. I killed Instance 2 and
started it again - it starts failing immediately. Which means the
descriptors are not closed.

> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index f7330af21d..93bf869299 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc> @@ -1938,50 +1996,63 @@ net_end_subscribe(struct cmsg *m)
> +
> +/**
> + * Create the session and invoke the on_connect triggers.
>   */
>  static void
>  tx_process_connect(struct cmsg *m)
>  {
>  	struct iproto_msg *msg = (struct iproto_msg *) m;
>  	struct iproto_connection *con = msg->connection;
> -	struct obuf *out = msg->connection->tx.p_obuf;
> -	try {              /* connect. */
> -		con->session = session_create(SESSION_TYPE_BINARY);
> -		if (con->session == NULL)
> -			diag_raise();
> -		con->session->meta.connection = con;
> -		tx_fiber_init(con->session, 0);
> -		char *greeting = (char *) static_alloc(IPROTO_GREETING_SIZE);
> -		/* TODO: dirty read from tx thread */
> -		struct tt_uuid uuid = INSTANCE_UUID;
> -		random_bytes(con->salt, IPROTO_SALT_SIZE);
> -		greeting_encode(greeting, tarantool_version_id(), &uuid,
> -				con->salt, IPROTO_SALT_SIZE);
> -		obuf_dup_xc(out, greeting, IPROTO_GREETING_SIZE);
> -		if (! rlist_empty(&session_on_connect)) {
> -			if (session_run_on_connect_triggers(con->session) != 0)
> -				diag_raise();
> -		}
> -		iproto_wpos_create(&msg->wpos, out);
> -	} catch (Exception *e) {
> +
> +	con->session = session_create(SESSION_TYPE_BINARY);
> +	if (con->session == NULL) {
>  		tx_reply_error(msg);
>  		msg->close_connection = true;
> +		return;
> +	}
> +	con->session->meta.connection = con;
> +
> +	tx_fiber_init(con->session, 0);
> +	if (! rlist_empty(&session_on_connect)) {

In the new code we don't use ' ' after unary operators.

> +		if (session_run_on_connect_triggers(con->session) != 0) {
> +			tx_reply_error(msg);
> +			msg->close_connection = true;
> +		}
>  	}
>  }

  reply	other threads:[~2020-12-21 16:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-19 17:39 [Tarantool-patches] [PATCH v7 0/3] iproto: greeting enhancement Ilya Kosarev
2020-12-19 17:39 ` [Tarantool-patches] [PATCH v7 1/3] iproto: move msg fields initialization to iproto_msg_new() Ilya Kosarev
2020-12-19 17:39 ` [Tarantool-patches] [PATCH v7 2/3] iproto: fix comment and add assert on destruction Ilya Kosarev
2020-12-19 17:39 ` [Tarantool-patches] [PATCH v7 3/3] iproto: move greeting from tx thread to iproto Ilya Kosarev
2020-12-21 16:20   ` Vladislav Shpilevoy [this message]
2020-12-22  0:50     ` Ilya Kosarev
2020-12-22  0:49 [Tarantool-patches] [PATCH v7 0/3] iproto: greeting enhancement Ilya Kosarev
2020-12-22  0:50 ` [Tarantool-patches] [PATCH v7 3/3] iproto: move greeting from tx thread to iproto Ilya Kosarev
2020-12-22 14:21   ` Vladislav Shpilevoy
2020-12-24 20:13     ` Ilya Kosarev
2020-12-26 13:15       ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d1eb7c2a-7afa-c2d9-9069-8a761d6aba86@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alyapunov@tarantool.org \
    --cc=i.kosarev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v7 3/3] iproto: move greeting from tx thread to iproto' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox