From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 423F5469710 for ; Wed, 10 Jun 2020 01:55:06 +0300 (MSK) References: <20200421213930.28713-1-gorcunov@gmail.com> From: Vladislav Shpilevoy Message-ID: Date: Wed, 10 Jun 2020 00:55:04 +0200 MIME-Version: 1.0 In-Reply-To: <20200421213930.28713-1-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] fio/coio: handle partial writes List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Hi! Thanks for the patch! See 4 comments below. On 21/04/2020 23:39, Cyrill Gorcunov wrote: > Writting less bytes than requested is perfectly fine. 1. 'Writting' -> 'Writing'. > 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. Note in most situations there > will be only one pass, partial writes are really the > rare cases. > > Fixes #4651 > > Signed-off-by: Cyrill Gorcunov > --- > issue https://github.com/tarantool/tarantool/issues/4651 > branch gorcunov/gh-4651-partial-write > > src/lib/core/coio_file.c | 62 ++++++++++++++++++++++++++++------ > src/lib/core/errinj.h | 1 + > test/app/fio.result | 73 ++++++++++++++++++++++++++++++++++++++++ > test/app/fio.test.lua | 21 ++++++++++++ > test/box/errinj.result | 1 + > 5 files changed, 147 insertions(+), 11 deletions(-) > > 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) > { > - INIT_COEIO_FILE(eio); > - eio_req *req = eio_write(fd, (void *) buf, count, offset, > - 0, coio_complete, &eio); > - return coio_wait_done(req, &eio); > + ssize_t left = count, pos = 0, res, chunk; > + eio_req *req; > + > + while (left > 0) { > + INIT_COEIO_FILE(eio); > + chunk = left; > + > + ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, { > + chunk = 1; > + }); > + > + req = eio_write(fd, (void *)buf + pos, chunk, 2. As it was fairly noticed by Timur in one of my patches for lsregion - lets better avoid 'void' pointer arithmetic. And use uint8_t * or char *. > + offset + pos, EIO_PRI_DEFAULT, > + coio_complete, &eio); > + res = coio_wait_done(req, &eio); > + if (res < 0) { > + pos = -1; 3. What if eio_write() returns EAGAIN/EINTR/WOULDBLOCK? Can it happen at all? Can it happen, if the descriptor is not blocking? Can it happen if the underlying FS is a network file system, like sshfs via FUSE? Coio socket library handles these errors and retries them. > + break; > + } else { > + left -= res; > + pos += res; > + } > + } > + return pos; > } > diff --git a/test/app/fio.result b/test/app/fio.result > index 783fa4fab..73fbd29e5 100644 > --- a/test/app/fio.result > +++ b/test/app/fio.result > @@ -865,6 +871,73 @@ fh6:close() > --- > - true > ... > +-- test partial write/pwrite via one byte transfer 4. Please - Add the issue reference, like this: -- -- gh-4651: ... -- - Start sentences from a capital letter, and finish with dot.