[Tarantool-patches] [PATCH v3] fio/coio: Handle partial writes
Alexander Turenko
alexander.turenko at tarantool.org
Fri Feb 21 17:48:58 MSK 2020
> > 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 at 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
More information about the Tarantool-patches
mailing list