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

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

> > On Wed, Dec 04, 2019 at 05:35:11PM +0300, Cyrill Gorcunov wrote:
> > > Writting less bytes than requested is fine. In turn our
> > > fio.write/pwrite api simply returns 'true' even if only
> > > some part of a buffer has been written. Thus make coio_write
> > > and coio_pwrite to write the whole data in a cycle.
> > 
> > I would expect that coio_*() functions will have the same behaviour as
> > its libc counterparts except that they'll yield a current fiber instead
> > of blocking a current thread.
> 
> They do yeild current fiber until completion notification arrives.
> coio_wait_done does exactly that.

I meant handling of a partial write: whether a function should
block/yield or report amount of written bytes to a caller.

So, my points here are the following:

* coio_write() should reflect write() in handling partial writes;
* fio lua module should handle partial writes on its own.

There is one interesting thing I found:

We have src/lib/core/fio.[ch], which handles partial writes, but
src/lua/fio.[ch] (Lua module) does not use it. It looks unusual.

> 
> > > 
> > > Fixes #4651
> > > 
> > > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> > > ---
> > > branch gorcunov/gh-4651-partial-write-3
> > 
> > > +	struct errinj *inj = errinj(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_INT);
> > > <...>
> > > +
> > > +		if (inj != NULL && inj->iparam > 0)
> > > +			chunk = (ssize_t)inj->iparam;
> > > +		else
> > > +			chunk = left;
> > > +
> > 
> > AFAIR, we have macros for error injections, which allow to avoid
> > producing any extra machine code for a release build. It seems they can
> > be used here. Can you elaborate, please?
> 
> I think I didn't use them to follow current inj code used
> in this file, for unification sake. But indeed maybe macors
> will look better. Will take a look.
> 
> > 
> > Nit: We usually split a cast operator from its argument with a
> > whitespace, e.g. `(ssize_t) inj->iparam`.
> 
> No we don't, there are number of examples where we put cast
> right before operand and I think it is a way more correct
> because code reader should consider such cast as signle entry
> together with operator itself. So I prefer this style.

You're right.

1. `man 1 indent` shows -ncs (--no-space-after-casts) in the Linux style
   description.
2. It is also copy-pasted to [1] without -npsl
   (--dont-break-procedure-type).
3. Vladimir D. uses it in this way in vinyl code.
4. The type cast operation has the same priority as other unary
   operations.

Don't know why I was sure that a space should be here.

[1]: https://github.com/tarantool/tarantool/wiki/Code-review-procedure-guidelines---checklist