Re: [Tarantool-patches] [PATCH] fio/coio: handle partial writes

[Alexander Turenko <alexander.turenko@tarantool.org>]

Since I have no other objections (but still not sure about a level where
it should be fixed), let's consider the patch LGTM for me, if Vlad will
agree with the proposed way to fix it.

WBR, Alexander Turenko.

On Fri, May 15, 2020 at 12:00:44PM +0300, Cyrill Gorcunov wrote:
> On Wed, May 06, 2020 at 08:08:18PM +0300, Alexander Turenko wrote:
> > > issue https://github.com/tarantool/tarantool/issues/4651
> > > branch gorcunov/gh-4651-partial-write
> > 
> > > diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
> > > index e2345567c..e290214bc 100644
> > > --- a/src/lib/core/coio_file.c
> > > +++ b/src/lib/core/coio_file.c
> > > @@ -164,10 +164,30 @@ coio_file_close(int fd)
> > >  ssize_t
> > >  coio_pwrite(int fd, const void *buf, size_t count, off_t offset)
> > >  {
> > 
> > In Febrary ([1]) we start discussing whether the loop should be in fio
> > or in coio. coio_p?write() returns amount of written bytes, so it seems
> > logical to keep it performing one write and move the loop to
> > src/lua/fio.c.
> > 
> > To be honest, I don't sure here. If you have a reason to keep the logic
> > here, please, explain it.
> 
> Because lua interface should not dive into low-level coio_ engine, moreover
> implementing it at this level we'are allowed to reuse coio_ helpers in
> future. In turn if we implement this on fio level we can't reuse it anywhere
> outside.
> 
> > I'll CC Vlad, maybe he has more strong vision here.
> > 
> > [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014403.html
> > 
> > > @@ -201,6 +221,11 @@ static void
> > >  coio_do_write(eio_req *req)
> > >  {
> > >  	struct coio_file_task *eio = (struct coio_file_task *)req->data;
> > > +
> > > +	ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, {
> > > +		eio->write.count = 1;
> > > +	});
> > 
> > Why not set it right in coio_write() to don't spread the logic?
> 
> Because the error should happen as close to native write call
> as possible to simulate real behaviour.
> 
> So I rebased the patch on latest master and pushed it back.