From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Tue, 11 Dec 2018 11:44:28 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH v2 03/11] sio: remove exceptions
Message-ID: <20181211084428.2ycn4h3vldjkuepr@esperanza>
References: <cover.1543958698.git.v.shpilevoy@tarantool.org>
 <fd79be8d0a088041d5a33dca9af6a5c78691159d.1543958698.git.v.shpilevoy@tarantool.org>
 <20181209125413.3zkcuchfltd4smph@esperanza>
 <1af45eb9-5b6e-9ea7-30d8-fc8ebcf37fa1@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1af45eb9-5b6e-9ea7-30d8-fc8ebcf37fa1@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Mon, Dec 10, 2018 at 06:37:37PM +0300, Vladislav Shpilevoy wrote:
> On 09/12/2018 15:54, Vladimir Davydov wrote:
> > On Wed, Dec 05, 2018 at 12:28:50AM +0300, Vladislav Shpilevoy wrote:
> > > @@ -40,6 +40,14 @@
> > >   #include "trivia/util.h"
> > >   #include "exception.h"
> > > +enum { SIO_ERROR_FATAL = -2 };
> > > +
> > > +bool
> > > +sio_is_error_fatal(int retcode)
> > > +{
> > > +	return retcode == SIO_ERROR_FATAL;
> > > +}
> > > +
> > >   /** Pretty print socket name and peer (for exceptions) */
> > >   const char *
> > >   sio_socketname(int fd)
> > > @@ -224,9 +234,11 @@ sio_accept(int fd, struct sockaddr *addr, socklen_t *addrlen)
> > >   {
> > >   	/* Accept a connection. */
> > >   	int newfd = accept(fd, addr, addrlen);
> > > -	if (newfd < 0 &&
> > > -	    (errno != EAGAIN && errno != EWOULDBLOCK && errno != EINTR))
> > > -		tnt_raise(SocketError, sio_socketname(fd), "accept");
> > > +	if (newfd < 0 && errno != EAGAIN && errno != EWOULDBLOCK &&
> > > +	    errno != EINTR) {
> > > +		diag_set(SocketError, sio_socketname(fd), "accept");
> > > +		return SIO_ERROR_FATAL;
> > > +	}
> > >   	return newfd;
> > >   }
> > 
> > This looks bad. It's not just my opinion. I consulted with Kostja
> > verbally and he agrees with me.
> > 
> > Let's please set diag on any error in all sio functions and let the
> > caller decide which ones are critical and which are not by checking
> 
> There is a problem - each diag increments error counter visible in
> stat. But I believe you know it, so as you wish.

Well, only ClientErrors are accounted in box.stat(), SocketErrors are
not. However, you're right - setting diag on each EWOULDBLOCK error does
seem to be expensive, because diag_set() implies malloc(). This would be
a pointless overhead.

So we've agreed that we shouldn't set diag on transient errors,
only errno. Kostja reworked and pushed this patch by himself.
For some reason, he also authored it, split it in 5, and did some
minor unnecessary changes (like moving comments).