From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Wed, 24 Oct 2018 12:54:18 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v2 2/4] wal: preallocate disk space before writing rows
Message-ID: <20181024095418.g22olfzylc6zaqs5@esperanza>
References: <cover.1540314925.git.vdavydov.dev@gmail.com>
 <484c921eb04a0a94563dcccf058bb5e8f96d3a49.1540314925.git.vdavydov.dev@gmail.com>
 <20181023183322.GB29694@chai>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181023183322.GB29694@chai>
To: Konstantin Osipov <kostja@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Tue, Oct 23, 2018 at 09:33:22PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/10/23 20:30]:
> > This function introduces a new xlog method xlog_fallocate() that makes
> > sure that the requested amount of disk space is available at the current
> > write position. It does that with posix_fallocate(). The new method is
> > called before writing anything to WAL, see wal_fallocate(). In order not
> > to invoke the system call too often, wal_fallocate() allocates disk
> > space in big chunks (1 MB).
> > 
> > The reason why I'm doing this is that I want to have a single and
> > clearly defined point in the code to handle ENOSPC errors, where
> > I could delete old WALs and retry (this is what #3397 is about).
> > 
> > Needed for #3397
> 
> Please consider comment about member names. 

OK, I'll try to take into account your input and come up with better
names in the next patch version.

> > @@ -1378,6 +1403,17 @@ xlog_write_eof(struct xlog *l)
> >  		diag_set(ClientError, ER_INJECTION, "xlog write injection");
> >  		return -1;
> >  	});
> > +
> > +	/*
> > +	 * Free disk space preallocated with xlog_fallocate().
> > +	 * Don't write the eof marker if this fails, otherwise
> > +	 * we'll get "data after eof marker" error on recovery.
> 
> I would simply remove this check - I don't see how it's useful as
> long as we never append to an existing xlog.
> 

Urgh, having a 1MB of 0 at the end of an xlog file looks ugly to me,
espcially if the xlog is small. IMO this doesn't increase the complexity
of the patch so we'd better leave it as is.

> > +	 */
> > +	if (l->alloc_len > 0 && ftruncate(l->fd, l->offset) < 0) {
> > +		diag_set(SystemError, "ftruncate() failed");
> > +		return -1;
> > +	}
> > +
> >  	if (fio_writen(l->fd, &eof_marker, sizeof(eof_marker)) < 0) {
> >  		diag_set(SystemError, "write() failed");
> >  		return -1;
> > @@ -1793,6 +1829,15 @@ xlog_cursor_next_tx(struct xlog_cursor *i)
> >  		return -1;
> >  	if (rc > 0)
> >  		return 1;
> > +	if (load_u32(i->rbuf.rpos) == 0) {
> > +		/*
> > +		 * Space preallocated with xlog_fallocate().
> > +		 * Treat as eof and clear the buffer.
> > +		 */
> > +		i->read_offset -= ibuf_used(&i->rbuf);
> > +		ibuf_reset(&i->rbuf);
> 
> Especially since you added this. 
> 
> > +		return 1;
> > +	}